Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid integer overflows with correct signing (tracking HB) and assertions #146

Merged
merged 2 commits into from
Nov 11, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions src/hb/ot_layout_gsubgpos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,9 @@ pub fn match_lookahead(
start_index: usize,
end_index: &mut usize,
) -> bool {
// Function should always be called with a non-zero starting index
// c.f. https://github.com/harfbuzz/rustybuzz/issues/142
assert!(start_index >= 1);
let mut iter = skipping_iterator_t::new(ctx, start_index - 1, true);
iter.set_glyph_data(0);
iter.enable_matching(match_func);
Expand Down Expand Up @@ -312,9 +315,9 @@ impl<'a, 'b> skipping_iterator_t<'a, 'b> {
}

pub fn prev(&mut self, unsafe_from: Option<&mut usize>) -> bool {
let stop = 0;
let stop: usize = 0;

while self.buf_idx > stop as usize {
while self.buf_idx > stop {
self.buf_idx -= 1;
let info = &self.ctx.buffer.out_info()[self.buf_idx];

Expand Down Expand Up @@ -858,7 +861,7 @@ fn apply_lookup(

// All positions are distance from beginning of *output* buffer.
// Adjust.
let mut end = {
let mut end: isize = {
let backtrack_len = ctx.buffer.backtrack_len();
let delta = backtrack_len as isize - ctx.buffer.idx as isize;

Expand All @@ -867,7 +870,7 @@ fn apply_lookup(
match_positions[j] = (match_positions[j] as isize + delta) as _;
}

backtrack_len + match_end - ctx.buffer.idx
backtrack_len as isize + match_end as isize - ctx.buffer.idx as isize
};

for record in lookups {
Expand Down Expand Up @@ -928,8 +931,8 @@ fn apply_lookup(
//
// It should be possible to construct tests for both of these cases.

end = end.saturating_add_signed(delta);
if end < match_positions[idx] {
end += delta;
if end < match_positions[idx] as isize {
// End might end up being smaller than match_positions[idx] if the recursed
// lookup ended up removing many items.
// Just never rewind end beyond start of current position, since that is
Expand All @@ -938,8 +941,8 @@ fn apply_lookup(
// https://bugs.chromium.org/p/chromium/issues/detail?id=659496
// https://github.com/harfbuzz/harfbuzz/issues/1611
//
delta += match_positions[idx] as isize - end as isize;
end = match_positions[idx];
delta += match_positions[idx] as isize - end;
end = match_positions[idx] as isize;
}

// next now is the position after the recursed lookup.
Expand Down Expand Up @@ -977,7 +980,7 @@ fn apply_lookup(
}
}

ctx.buffer.move_to(end);
ctx.buffer.move_to(end.try_into().unwrap());
}

/// Value represents glyph class.
Expand Down Expand Up @@ -1316,6 +1319,9 @@ pub fn ligate_input(
if this_comp == 0 {
this_comp = last_num_comps;
}
// Avoid the potential for a wrap-around bug when subtracting from an unsigned integer
// c.f. https://github.com/harfbuzz/rustybuzz/issues/142
assert!(comps_so_far >= last_num_comps);
let new_lig_comp = comps_so_far - last_num_comps + this_comp.min(last_num_comps);
_hb_glyph_info_set_lig_props_for_mark(cur, lig_id, new_lig_comp);
}
Expand Down Expand Up @@ -1344,6 +1350,9 @@ pub fn ligate_input(
break;
}

// Avoid the potential for a wrap-around bug when subtracting from an unsigned integer
// c.f. https://github.com/harfbuzz/rustybuzz/issues/142
assert!(comps_so_far >= last_num_comps);
let new_lig_comp = comps_so_far - last_num_comps + this_comp.min(last_num_comps);
_hb_glyph_info_set_lig_props_for_mark(info, lig_id, new_lig_comp)
}
Expand Down
Loading