GoogleCodeExporter
commented
9 years ago Reported in https://savannah.nongnu.org/bugs/?43539.Original comment by mjurc...@google.com on 5 Nov 2014 at 2:06
- Added labels: ****
- Removed labels: ****
Closed GoogleCodeExporter closed 9 years ago
GoogleCodeExporter
commented
9 years ago Reported in https://savannah.nongnu.org/bugs/?43539.Original comment by mjurc...@google.com on 5 Nov 2014 at 2:06
GoogleCodeExporter
commented
9 years ago Original comment by mjurc...@google.com on 5 Nov 2014 at 2:07
GoogleCodeExporter
commented
9 years ago The developer tried to fix this in
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/?id=35252ae9aa1dd
9343e9f4884e9ddb1fee10ef415, but the vulnerability can still be triggered for
very large files (> 2GB).
I'm pinging the developer to complement the fix with an additional check.Original comment by mjurc...@google.com on 26 Nov 2014 at 12:03
GoogleCodeExporter
commented
9 years ago In fact, when
https://code.google.com/p/google-security-research/issues/detail?id=153 is
properly fixed, the issue should be resolved. I'm marking this one as fixed,
then.Original comment by mjurc...@google.com on 26 Nov 2014 at 12:07
GoogleCodeExporter
commented
9 years ago All fixed by upstream:
FreeType 2.5.5
2014-12-30
FreeType 2.5.5 has been released. This is a minor bug fix release: All users of
PCF fonts should update, since version 2.5.4 introduced a bug that prevented
reading of such font files if not compressed.
FreeType 2.5.4
2014-12-06
FreeType 2.5.4 has been released. All users should upgrade due to another fix
for vulnerability CVE-2014-2240 in the CFF driver. The library also contains a
new round of patches for better protection against malformed fonts.
The main new feature, which is also one of the targets mentioned in the pledgie
roadmap below, is auto-hinting support for Devanagari and Telugu, two widely
used Indic scripts. A more detailed description of the remaining changes and
fixes can be found here.
Original comment by cev...@google.com on 26 Jan 2015 at 5:27
GoogleCodeExporter
commented
9 years ago Information in this issue does not seem to be correct.
This is the version of the code form the time of the report (early Nov 2014):
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/base/ftobjs.c?i
d=bbd8313#n1574
Description above quotes the following code:
1628: if ( FT_READ_LONG( rlen ) )
1629: goto Exit;
...
1676: if ( pfb_pos > pfb_len || pfb_pos + rlen > pfb_len )
1677: goto Exit2;
1678:
1679: error = FT_Stream_Read( stream, (FT_Byte *)pfb_data + pfb_pos, rlen
);
It does not quote another important code in between the above lines:
1639 /* the flags are part of the resource, so rlen >= 2. */
1640 /* but some fonts declare rlen = 0 for empty fragment */
1641 if ( rlen > 2 )
1642 rlen -= 2;
1643 else
1644 rlen = 0;
Hence 'If the value of "rlen" is negative, the check is bypassed' should never
happen, as rlen can not be negative on line 1676.
The rlen value in the attached test case is 0x7fffffff, which is positive.
AFAICS, this is not a separate issue, but a different test case for the integer
overflow problems described in issue #153.Original comment by tho...@redhat.com on 24 Feb 2015 at 8:16
GoogleCodeExporter
commented
9 years ago The part that probably can be considered to not be part of issue #153 is
handling of temp. As that is signed, it can reduce pfb_len counter value
without actually making it wrap around. When it's re-read as rlen, excessively
long read/write in FT_Stream_Read() is avoided by the rlen check mentioned in
comment #6 above, but having it tagged as comment would be another way.Original comment by tho...@redhat.com on 24 Feb 2015 at 8:45
GoogleCodeExporter
commented
9 years ago Original comment by mjurc...@google.com on 25 Feb 2015 at 2:06
GoogleCodeExporter
commented
9 years ago @thoger, I have looked at the source code again and I believe you are right -
it was not possible for a negative "rlen" to slip through up to lines 1676 -
1679 because of the if() statement in between, so the problem is not really as
originally described.
However, there were still multiple integer signedness handling problems in the
quoted fragments of code, which might or might not have been
mitigated/prevented by fixes from issue 153. The intent in creating two issues
was to separate the wrap-around integer overflows (issue 153) and the
signedness problems in sanity checking performed in the code (this issue). It
has also resulted in converting the local variables in the function to unsigned
types and introducing more in-depth bounds checking, so I am leaving the bug as
is.Original comment by mjurc...@google.com on 25 Feb 2015 at 2:46
Original issue reported on code.google.com by
mjurc...@google.comon 5 Nov 2014 at 2:03Attachments: