Open Bug 1368861 Opened 7 years ago Updated 2 years ago

Graphite2: multiple integer overflows

Categories

(Core :: Graphics: Text, defect, P3)

defect

Tracking

()

People

(Reporter: tsmith, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-intoverflow, testcase, Whiteboard: [gfx-noted])

Attachments

(1 file)

1.62 KB, application/x-font-ttf
Details
Attached file test_case.ttf
These were found using a 32bit build with Undefined behavior sanitizer. Graphite git commit 090076bf4.

Martin can you please have a look when you have a moment? I'm not sure how many of these are intended. From time to time large memory allocations will trigger crashes while fuzzing so cleaning these up when possible would be helpful.

To reproduce build with -DCMAKE_C_FLAGS="-m32 -fsanitize=integer" -DCMAKE_CXX_FLAGS="-m32 -fsanitize=integer"

./gr2fonttest -auto -noprint test_case.ttf
graphite/src/inc/bits.h:84:18: runtime error: unsigned integer overflow: 512 * 16843009 cannot be represented in type 'unsigned long'
    #0 0x818694b in unsigned int graphite2::bit_set_count<unsigned long>(unsigned long) graphite/src/inc/bits.h:84:18
    #1 0x818694b in graphite2::sparse::capacity() const graphite/src/Sparse.cpp:59
    #2 0x8158789 in graphite2::GlyphCache::Loader::read_glyph(unsigned short, graphite2::GlyphFace&, int*) const graphite/src/GlyphCache.cpp:429:31
    #3 0x8154d1d in graphite2::GlyphCache::GlyphCache(graphite2::Face const&, unsigned int) graphite/src/GlyphCache.cpp:135:22
    #4 0x814acb6 in graphite2::Face::readGlyphs(unsigned int) graphite/src/Face.cpp:98:29
    #5 0x8140d76 in (anonymous namespace)::load_face(graphite2::Face&, unsigned int) graphite/src/gr_face.cpp:54:14
    #6 0x8140d76 in gr_make_face_with_ops graphite/src/gr_face.cpp:89
    #7 0x8142418 in gr_make_file_face graphite/src/gr_face.cpp:242:23
    #8 0x813b379 in Parameters::testFileFont() const graphite/gr2fonttest/gr2FontTest.cpp:639:20
    #9 0x813de82 in main graphite/gr2fonttest/gr2FontTest.cpp:802:9
    #10 0xf7539636 in __libc_start_main /build/glibc-5sb1ri/glibc-2.23/csu/../csu/libc-start.c:291
    #11 0x805fd67 in _start (gr2fonttest+0x805fd67)

graphite/src/TtfUtil.cpp:1004:37: runtime error: unsigned integer overflow: 4294967266 + 32 cannot be represented in type 'unsigned int'
    #0 0x818a192 in graphite2::TtfUtil::CmapSubtable4Lookup(void const*, unsigned int, int) graphite/src/TtfUtil.cpp:1004:37
    #1 0x8196f61 in bool cache_subtable<&graphite2::TtfUtil::CmapSubtable4NextCodepoint, &graphite2::TtfUtil::CmapSubtable4Lookup>(unsigned short**, void const*, unsigned int) graphite/src/CmapCache.cpp:76:43
    #2 0x8195921 in graphite2::CachedCmap::CachedCmap(graphite2::Face const&) graphite/src/CmapCache.cpp:107:14
    #3 0x814ae81 in graphite2::Face::readGlyphs(unsigned int) graphite/src/Face.cpp:108:22
    #4 0x8140d76 in (anonymous namespace)::load_face(graphite2::Face&, unsigned int) graphite/src/gr_face.cpp:54:14
    #5 0x8140d76 in gr_make_face_with_ops graphite/src/gr_face.cpp:89
    #6 0x8142418 in gr_make_file_face graphite/src/gr_face.cpp:242:23
    #7 0x813b379 in Parameters::testFileFont() const graphite/gr2fonttest/gr2FontTest.cpp:639:20
    #8 0x813de82 in main graphite/gr2fonttest/gr2FontTest.cpp:802:9
    #9 0xf7539636 in __libc_start_main /build/glibc-5sb1ri/glibc-2.23/csu/../csu/libc-start.c:291
    #10 0x805fd67 in _start (gr2fonttest+0x805fd67)

graphite/src/inc/bits.h:84:18: runtime error: unsigned integer overflow: 134744072 * 16843009 cannot be represented in type 'unsigned int'
    #0 0x814ee15 in unsigned int graphite2::bit_set_count<unsigned int>(unsigned int) graphite/src/inc/bits.h:84:18
    #1 0x814ee15 in graphite2::FeatureRef::FeatureRef(graphite2::Face const&, unsigned short&, unsigned int, unsigned int, unsigned short, unsigned short, graphite2::FeatureSetting*, unsigned short) graphite/src/FeatureMap.cpp:85
    #2 0x814f6b8 in graphite2::FeatureMap::readFeats(graphite2::Face const&) graphite/src/FeatureMap.cpp:165:29
    #3 0x8150f26 in graphite2::SillMap::readFace(graphite2::Face const&) graphite/src/FeatureMap.cpp:191:10
    #4 0x814c2f4 in graphite2::Face::readFeatures() graphite/src/Face.cpp:161:12
    #5 0x8140ddd in (anonymous namespace)::load_face(graphite2::Face&, unsigned int) graphite/src/gr_face.cpp:59:18
    #6 0x8140ddd in gr_make_face_with_ops graphite/src/gr_face.cpp:89
    #7 0x8142418 in gr_make_file_face graphite/src/gr_face.cpp:242:23
    #8 0x813b379 in Parameters::testFileFont() const graphite/gr2fonttest/gr2FontTest.cpp:639:20
    #9 0x813de82 in main graphite/gr2fonttest/gr2FontTest.cpp:802:9
    #10 0xf7539636 in __libc_start_main /build/glibc-5sb1ri/glibc-2.23/csu/../csu/libc-start.c:291
    #11 0x805fd67 in _start (gr2fonttest+0x805fd67)

graphite/src/inc/bits.h:84:18: runtime error: unsigned integer overflow: 1032 * 16843009 cannot be represented in type 'unsigned int'
    #0 0x81683a6 in unsigned int graphite2::bit_set_count<unsigned int>(unsigned int) graphite/src/inc/bits.h:84:18
    #1 0x81683a6 in unsigned int graphite2::log_binary<unsigned int>(unsigned int) graphite/src/inc/bits.h:122
    #2 0x81683a6 in graphite2::Segment::Segment(unsigned int, graphite2::Face const*, unsigned int, int) graphite/src/Segment.cpp:63
    #3 0x81438d7 in (anonymous namespace)::makeAndInitialize(graphite2::Font const*, graphite2::Face const*, unsigned int, graphite2::FeatureVal const*, gr_encform, void const*, unsigned int, int) graphite/src/gr_segment.cpp:43:25
    #4 0x81438d7 in gr_make_seg graphite/src/gr_segment.cpp:105
    #5 0x813b8b0 in Parameters::testFileFont() const graphite/gr2fonttest/gr2FontTest.cpp:692:20
    #6 0x813de82 in main graphite/gr2fonttest/gr2FontTest.cpp:802:9
    #7 0xf7539636 in __libc_start_main /build/glibc-5sb1ri/glibc-2.23/csu/../csu/libc-start.c:291
    #8 0x805fd67 in _start (gr2fonttest+0x805fd67)
There are 2 issues for consideration here.

1. bits.h. The fast calculation of the number of bits in an integer uses a standard bit twiddling technique that involves an overflow multiplication regardless of integer size.

2. cmap delta entry. This warning arises due to the slightly odd auto casting, but is not dangerous. The uint32 coerces the int16 into a sign extended uint32 and hence the overflow on what should be a signed calculation. Which all falls out of the wash into a uint16. The rest of the engine is used to bad return values from this.

So. It might be possible to make issue 2 less noisy. But there is nothing securityish here. Thanks for trying :)
OK thanks, no reason to keep this closed.
Group: gfx-core-security
Whiteboard: [gfx-noted]
Has Regression Range: --- → no
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.