[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Review graphite2



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Hello.

I prepared LTS security update for graphite2[1]. Debdiff is attached.
All tests ran successfully. Please review.



- -abhijith

[1]
https://mentors.debian.net/debian/pool/main/g/graphite2/graphite2_1.3.10
- -1~deb7u2.dsc
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEE7xPqJqaY/zX9fJAuhj1N8u2cKO8FAlquArUACgkQhj1N8u2c
KO8j3A//QeA2nOPNG/j2GJ+j+LSKWRt/khRoL2vY2YpXcIRCJSACCVZT+LqOezLF
BfMTa6TEeu9zXfRQpuAoHBZtOmSbPaldi6flT5eJ469mF/tqBY7Bdx7us5/bmni0
YxAz7tYp6oU3hBUz2HqEgH293cm7+wG7mLjSGG5EVcuFCIwRud0Y7/s0YQV7/xJ6
9YBUnzUPCZ/h0jcZNXpUmo2QWtvvaFj1vg5KQQ3JvKGGdVly9cJse+E1Z8FLih46
4ZNCNMjF3AWnn2MyVk1b9Ej8kr69CsrZqxkRpnVovsg2N7VUuwp+SiYndlBfqTvu
MIr84/NPfCG9F7V8kyO486QRsB8fHYGA4+HnTL/iGZYgEIeRJgIyAQqaOGRhhrgU
NASuJydVTtRiVQuL9mrx/S6lfUFTaYRRGMm7SagDxeHN1wR1SuXxjwm4LOqrRzHD
eR4AxYJmnu9iMZkYaYIsy9VfdimAF63l8mCfVEede1zuug12YunWjUQkZA1xRnjM
xD67lo2RQWx6FMb7uiLt6/EUP6VouXoVJi2jt/BBpXgx66gWLQOb2xvDQAI3pQP2
C4AbI9H+Uvyjbcufe1GusKXkBGvny3LWkQiAwuScfkUMNlhemSsc83wy4jRTgRyb
NGfWN85t8eQMlrviVhdZz0YcFjBMRx5qe9iA+nUL5eANbKSz2RY=
=SOWG
-----END PGP SIGNATURE-----
diff -Nru graphite2-1.3.10/debian/changelog graphite2-1.3.10/debian/changelog
--- graphite2-1.3.10/debian/changelog	2017-07-04 20:48:57.000000000 +0530
+++ graphite2-1.3.10/debian/changelog	2018-03-17 08:44:25.000000000 +0530
@@ -1,3 +1,11 @@
+graphite2 (1.3.10-1~deb7u2) wheezy-security; urgency=high
+
+  * Non-maintainer upload by the LTS Team.
+  * Fix CVE-2018-7999: NULL pointer dereference vulnerability
+    (closes: #892590)
+
+ -- Abhijith PA <abhijith@disroot.org>  Sat, 17 Mar 2018 08:44:25 +0530
+
 graphite2 (1.3.10-1~deb7u1) wheezy-security; urgency=high
 
   * Non-maintainer upload by the LTS team.
diff -Nru graphite2-1.3.10/debian/patches/CVE-2018-7999.patch graphite2-1.3.10/debian/patches/CVE-2018-7999.patch
--- graphite2-1.3.10/debian/patches/CVE-2018-7999.patch	1970-01-01 05:30:00.000000000 +0530
+++ graphite2-1.3.10/debian/patches/CVE-2018-7999.patch	2018-03-17 08:44:25.000000000 +0530
@@ -0,0 +1,208 @@
+Description: Fix CVE-2018-7999
+ a NULL pointer dereference vulnerability was found in Segment.cpp during a 
+ dumbRendering operation, which may allow attackers to cause a denial of service
+ or possibly have unspecified other impact via a crafted .ttf file.
+
+Author: Abhijith PA <abhijith@disroot.org>
+Origin: https://github.com/silnrsi/graphite/commit/db132b4731a9b4c9534144ba3a18e65b390e9ff6
+Bug: https://github.com/silnrsi/graphite/issues/22
+Bug-Debian: https://bugs.debian.org/892590
+Last-Update: 2018-03-18
+
+--- graphite2-1.3.10.orig/src/GlyphCache.cpp
++++ graphite2-1.3.10/src/GlyphCache.cpp
+@@ -84,7 +84,7 @@ const SlantBox SlantBox::empty = {0,0,0,
+ class GlyphCache::Loader
+ {
+ public:
+-    Loader(const Face & face, const bool dumb_font);    //return result indicates success. Do not use if failed.
++    Loader(const Face & face);    //return result indicates success. Do not use if failed.
+ 
+     operator bool () const throw();
+     unsigned short int units_per_em() const throw();
+@@ -115,7 +115,7 @@ private:
+ 
+ 
+ GlyphCache::GlyphCache(const Face & face, const uint32 face_options)
+-: _glyph_loader(new Loader(face, bool(face_options & gr_face_dumbRendering))),
++: _glyph_loader(new Loader(face)),
+   _glyphs(_glyph_loader && *_glyph_loader && _glyph_loader->num_glyphs()
+         ? grzeroalloc<const GlyphFace *>(_glyph_loader->num_glyphs()) : 0),
+   _boxes(_glyph_loader && _glyph_loader->has_boxes() && _glyph_loader->num_glyphs()
+@@ -239,7 +239,7 @@ const GlyphFace *GlyphCache::glyph(unsig
+ 
+ 
+ 
+-GlyphCache::Loader::Loader(const Face & face, const bool dumb_font)
++GlyphCache::Loader::Loader(const Face & face)
+ : _head(face, Tag::head),
+   _hhea(face, Tag::hhea),
+   _hmtx(face, Tag::hmtx),
+@@ -265,52 +265,49 @@ GlyphCache::Loader::Loader(const Face &
+         return;
+     }
+ 
+-    if (!dumb_font)
++    if ((m_pGlat = Face::Table(face, Tag::Glat, 0x00030000)) == NULL
++        || (m_pGloc = Face::Table(face, Tag::Gloc)) == NULL
++        || m_pGloc.size() < 8)
+     {
+-        if ((m_pGlat = Face::Table(face, Tag::Glat, 0x00030000)) == NULL
+-            || (m_pGloc = Face::Table(face, Tag::Gloc)) == NULL
+-            || m_pGloc.size() < 8)
+-        {
+-            _head = Face::Table();
+-            return;
+-        }
+-        const byte    * p = m_pGloc;
+-        int       version = be::read<uint32>(p);
+-        const uint16    flags = be::read<uint16>(p);
+-        _num_attrs = be::read<uint16>(p);
+-        // We can accurately calculate the number of attributed glyphs by
+-        //  subtracting the length of the attribids array (numAttribs long if present)
+-        //  and dividing by either 2 or 4 depending on shor or lonf format
+-        _long_fmt              = flags & 1;
+-        int tmpnumgattrs       = (m_pGloc.size()
+-                                   - (p - m_pGloc)
+-                                   - sizeof(uint16)*(flags & 0x2 ? _num_attrs : 0))
+-                                       / (_long_fmt ? sizeof(uint32) : sizeof(uint16)) - 1;
+-
+-        if (version >= 0x00020000 || tmpnumgattrs < 0 || tmpnumgattrs > 65535
+-            || _num_attrs == 0 || _num_attrs > 0x3000  // is this hard limit appropriate?
+-            || _num_glyphs_graphics > tmpnumgattrs
+-            || m_pGlat.size() < 4)
+-        {
+-            _head = Face::Table();
+-            return;
+-        }
++        _head = Face::Table();
++        return;
++    }
++    const byte    * p = m_pGloc;
++    int       version = be::read<uint32>(p);
++    const uint16    flags = be::read<uint16>(p);
++    _num_attrs = be::read<uint16>(p);
++    // We can accurately calculate the number of attributed glyphs by
++    //  subtracting the length of the attribids array (numAttribs long if present)
++    //  and dividing by either 2 or 4 depending on shor or lonf format
++    _long_fmt              = flags & 1;
++    int tmpnumgattrs       = (m_pGloc.size()
++                               - (p - m_pGloc)
++                               - sizeof(uint16)*(flags & 0x2 ? _num_attrs : 0))
++                                   / (_long_fmt ? sizeof(uint32) : sizeof(uint16)) - 1;
++
++    if (version >= 0x00020000 || tmpnumgattrs < 0 || tmpnumgattrs > 65535
++        || _num_attrs == 0 || _num_attrs > 0x3000  // is this hard limit appropriate?
++        || _num_glyphs_graphics > tmpnumgattrs
++        || m_pGlat.size() < 4)
++    {
++        _head = Face::Table();
++        return;
++    }
+ 
+         _num_glyphs_attributes = static_cast<unsigned short>(tmpnumgattrs);
+-        p = m_pGlat;
+-        version = be::read<uint32>(p);
+-        if (version >= 0x00040000 || (version >= 0x00030000 && m_pGlat.size() < 8))       // reject Glat tables that are too new
+-        {
+-            _head = Face::Table();
+-            return;
+-        }
+-        else if (version >= 0x00030000)
+-        {
+-            unsigned int glatflags = be::read<uint32>(p);
+-            _has_boxes = glatflags & 1;
+-            // delete this once the compiler is fixed
+-            _has_boxes = true;
+-        }
++    p = m_pGlat;
++    version = be::read<uint32>(p);
++    if (version >= 0x00040000 || (version >= 0x00030000 && m_pGlat.size() < 8))       // reject Glat tables that are too new
++    {
++        _head = Face::Table();
++        return;
++    }
++    else if (version >= 0x00030000)
++    {
++        unsigned int glatflags = be::read<uint32>(p);
++        _has_boxes = glatflags & 1;
++        // delete this once the compiler is fixed
++        _has_boxes = true;
+     }
+ }
+ 
+--- graphite2-1.3.10.orig/src/gr_face.cpp
++++ graphite2-1.3.10/src/gr_face.cpp
+@@ -47,8 +47,7 @@ namespace
+         telemetry::category _misc_cat(face.tele.misc);
+ #endif
+         Face::Table silf(face, Tag::Silf, 0x00050000);
+-        if (silf)   options &= ~gr_face_dumbRendering;
+-        else if (!(options &  gr_face_dumbRendering))
++        if (!silf)
+             return false;
+ 
+         if (!face.readGlyphs(options))
+@@ -74,7 +73,7 @@ namespace
+                 return true;
+         }
+         else
+-            return options & gr_face_dumbRendering;
++            return false;
+     }
+ }
+ 
+--- graphite2-1.3.10.orig/tests/featuremap/CMakeLists.txt
++++ graphite2-1.3.10/tests/featuremap/CMakeLists.txt
+@@ -20,7 +20,7 @@ if (GRAPHITE2_ASAN)
+ endif (GRAPHITE2_ASAN)
+ target_link_libraries(featuremaptest graphite2 graphite2-base graphite2-segcache graphite2-base)
+ 
+-add_test(NAME featuremaptest COMMAND $<TARGET_FILE:featuremaptest> ${testing_SOURCE_DIR}/fonts/tiny.ttf)
++add_test(NAME featuremaptest COMMAND $<TARGET_FILE:featuremaptest> ${testing_SOURCE_DIR}/fonts/small.ttf)
+ set_tests_properties(featuremaptest PROPERTIES TIMEOUT 3)
+ if (GRAPHITE2_ASAN)
+     set_property(TEST featuremaptest APPEND PROPERTY ENVIRONMENT "ASAN_SYMBOLIZER_PATH=${ASAN_SYMBOLIZER}")
+--- graphite2-1.3.10.orig/tests/featuremap/featuremaptest.cpp
++++ graphite2-1.3.10/tests/featuremap/featuremaptest.cpp
+@@ -243,7 +243,7 @@ template <class T> void testFeatTable(co
+ {
+     FeatureMap testFeatureMap;
+     dummyFace.replace_table(TtfUtil::Tag::Feat, &table, sizeof(T));
+-    gr_face * face = gr_make_face_with_ops(&dummyFace, &face_handle::ops, gr_face_dumbRendering);
++    gr_face * face = gr_make_face_with_ops(&dummyFace, &face_handle::ops, 0);
+     if (!face) throw std::runtime_error("failed to load font");
+     bool readStatus = testFeatureMap.readFeats(*face);
+     testAssert("readFeats", readStatus);
+@@ -285,9 +285,8 @@ int main(int argc, char * argv[])
+ 		// test a bad settings offset stradling the end of the table
+ 		FeatureMap testFeatureMap;
+ 		dummyFace.replace_table(TtfUtil::Tag::Feat, &testBadOffset, sizeof testBadOffset);
+-		face = gr_make_face_with_ops(&dummyFace, &face_handle::ops, gr_face_dumbRendering);
+-		bool readStatus = testFeatureMap.readFeats(*face);
+-		testAssert("fail gracefully on bad table", !readStatus);
++		face = gr_make_face_with_ops(&dummyFace, &face_handle::ops, 0);
++		testAssert("fail gracefully on bad table", !face);
+ 	}
+ 	catch (std::exception & e)
+ 	{
+--- graphite2-1.3.10.orig/tests/vm/CMakeLists.txt
++++ graphite2-1.3.10/tests/vm/CMakeLists.txt
+@@ -41,7 +41,7 @@ if  (${CMAKE_SYSTEM_NAME} STREQUAL "Linu
+ 	endif ("${CMAKE_BUILD_TYPE}" STREQUAL "Release")
+ endif  (${CMAKE_SYSTEM_NAME} STREQUAL "Linux" OR ${CMAKE_SYSTEM_NAME} MATCHES "k.*BSD" OR ${CMAKE_SYSTEM_NAME} STREQUAL "GNU")
+ 
+-add_test(vm-test-call-threading vm-test-call ${testing_SOURCE_DIR}/fonts/tiny.ttf 1)
++add_test(vm-test-call-threading vm-test-call ${testing_SOURCE_DIR}/fonts/small.ttf 1)
+ set_tests_properties(vm-test-call-threading PROPERTIES
+         PASS_REGULAR_EXPRESSION "simple program size:    14 bytes.*result of program: 42"
+         FAIL_REGULAR_EXPRESSION "program terminated early;stack not empty")
+@@ -51,7 +51,7 @@ if (GRAPHITE2_ASAN)
+ endif (GRAPHITE2_ASAN)
+ 
+ if  (${CMAKE_COMPILER_IS_GNUCXX})
+-	add_test(vm-test-direct-threading vm-test-direct ${testing_SOURCE_DIR}/fonts/tiny.ttf 1)
++	add_test(vm-test-direct-threading vm-test-direct ${testing_SOURCE_DIR}/fonts/small.ttf 1)
+ 	set_tests_properties(vm-test-direct-threading PROPERTIES
+ 			PASS_REGULAR_EXPRESSION "simple program size:    14 bytes.*result of program: 42"
+ 			FAIL_REGULAR_EXPRESSION "program terminated early;stack not empty")
diff -Nru graphite2-1.3.10/debian/patches/series graphite2-1.3.10/debian/patches/series
--- graphite2-1.3.10/debian/patches/series	2017-07-04 20:48:57.000000000 +0530
+++ graphite2-1.3.10/debian/patches/series	2018-03-17 08:44:25.000000000 +0530
@@ -5,3 +5,4 @@
 test-timeout.diff
 reproducible-build.diff
 revert-to-old-SONAME.diff
+CVE-2018-7999.patch

Attachment: graphite2_deb7u2.debdiff.sig
Description: PGP signature


Reply to: