Skip to content

Fix out-of-bounds access in cff subset

Uli Schlachter requested to merge psychon/cairo:oob-cff-subset into master

I was looking at 1. While trying to reproduce the problem that is described there, valgrind reported:

Argument 'size' of function malloc has a fishy (possibly negative) value: -8 at 0x48407B4: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x4B20E92: cairo_cff_font_read_name (cairo-cff-subset.c:895) by 0x4B221AD: cairo_cff_font_read_font (cairo-cff-subset.c:1351) by 0x4B24EF2: cairo_cff_font_generate (cairo-cff-subset.c:2587) by 0x4B25EA3: _cairo_cff_subset_init (cairo-cff-subset.c:2979)

This commit is about fixing the above.

The function decode_index_offset() returns an unsigned long. This value was cast to an "int" in cff_index_read(), leading to a possibility for over/underflow. Also, nothing checked that an entry in the index table had a non-zero length, leading to an entry with length -8 as reported by valgrind.

Fix this by using "unsigned long" for the local variables and checking the length to be non-negative.

With the above fixed, the original test case started crashing. Apparently, cairo_cff_font_read_name() does not expect nor handle failures from cff_index_read(). Thus, a check for this case was added to make the new crash go away.

Signed-off-by: Uli Schlachter psychon@znc.in

Fix a possible out-of-bounds read

While working on the previous commit, I noticed that nothing makes sure that the entry points within the font data. Thus, this could easily cause out-of-bounds reads.

This commit adds a suitable length check for this.

Merge request reports