Mon 05 Aug 2013 06:32:09 PM UTC, original submission:
I'm not sure if there is still interest in reporting issues in the 'old' cff code but in case there is :
The offset values in a CFF INDEX structure are being sanity checked in cff_index_get_pointers while building an equivalent pointer array.
cur_offset = idx->offsets[0] - 1;
/* sanity check */
if ( cur_offset >= idx->data_size )
{
FT_TRACE0(( "cff_index_get_pointers:"
" invalid first offset value %d set to zero\n",
cur_offset is the first offset to be found in the INDEX structure minus one. By definition this needs to be value 0 (cfr. Adobe TechNote 5176, section 5). So this test can be made stronger by checking for cur_offset != 0.
/* empty slot + two sanity checks for invalid offset tables */
if ( next_offset == 0 ||
next_offset < cur_offset ||
( next_offset >= idx->data_size && n < idx->count ) )
These tests are not 100% correct. Basically you want to protect the code using the pointer array later on so that it can rely on monotonic increasing pointer values so that cff_decoder_parse_charstrings() can always have 'ip' growing towards 'limit', cfr. its local/global subrs handling.
The first subtest 'next_offset == 0' is already covered by 'next_offset < cur_offset' as the type is unsigned, so that can be dropped. However, the third subtest is actually wrong for INDEX structures which have one or more empty entries at the end as the last non-empty entry will actually also made empty as it matches the 'next_offset >= idx->data_size && n < idx->count' subtest.
I believe these can be better replaced by:
if ( next_offset < cur_offset )
next_offset = cur_offset;
else if ( next_offset > idx->data_size )
next_offset = idx->data_size;
I.e. it just clamps the next_offset between the current offset and the maximum value it can ever have.
I've attached a diff covering those changes and also a subset font which wrong triggers the 'next_offset >= idx->data_size && n < idx->count' subtest for its gid 66 glyph.
|