On 17 Feb, Andrea Pescetti wrote: > Pedro Giffuni wrote: >> I looked briefly at the issues and for good or bad the version of >> silgraphite shipping with OpenOffice is old enough that most of the >> vulnerabilities don't apply (at least not directly). > > Thank you for setting things straight. We do use silgrahpite, but not > the version that is confirmed to be vulnerable. > > Indeed the article you linked to does not say that OpenOffice is > vulnerable. It says that OpenOffice uses silgraphite (correct) and that > Firefox used to be vulnerable (since Firefox was using the silgraphite > version that is confirmed to be vulnerable). > >> 1) We could update silgraphite to their latest version (at least on >> header has disappeared so this needs tweaking). >> 2) We could patch the older silgraphite to provide some protection >> from vulnerabilities. > > I would definitely go for option 1 but indeed they broke compatibility. > I don't know how complex it is to update code, but it is a good moment > for doing so.
Option 1 would be my preference, but it looks non-trivial. >> Independent of (1) or (2) I think it's likely we may want to stop >> shipping libgraphite. > > I don't think this is the best solution, see below. > >> One one side the support from SIL for this >> event has been unacceptable: AFAICT there was no advance notice > > I confirm OpenOffice received no information in advance; on the other > side, the vulnerability as such does not apply to the version we use. So > maybe we didn't receive a notification since there was nothing to fix. > >> On the other hand graphite is not very important >> nowadays: Adobe donated a fine CFF rasterizer to the freetype >> project which fills the hole graphite meant to cover. > > We do have a niche (at least I think it's a niche) of users who love > Graphite-enabled fonts. So this might need some longer evaluation, at > least to understand if these users would be damaged. This is why I would > prefer to use option 1 for 4.2.0 and (unless they broke compatibility > too much) go for the update. Of course, if this turns out to be too > complex or risky, deprecating silgrahpite is an option too. After looking at the code, it appears that the version of silgraphite that we are using is subject to one of the four vulnerabilities mentioned: <http://www.talosintel.com/reports/TALOS-2016-0061/>. The patch below is similar to the upstream graphite2 fix and I believe it closes the hole. I've tested it with the GentiumPlus font and it didn't break anything obvious. One concern that I have is that silgraphite has been deprecated upstream and its last release was in February 2009, so there may be quite a few old bugs lurking there. --- engine/src/font/TtfUtil.cpp.orig 2009-01-29 08:33:19 UTC +++ engine/src/font/TtfUtil.cpp @@ -1106,7 +1106,7 @@ size_t LocaLookup(gr::gid16 nGlyphId, // CheckTable verifies the index_to_loc_format is valid if (read(pTable->index_to_loc_format) == Sfnt::FontHeader::ShortIndexLocFormat) { // loca entries are two bytes and have been divided by two - if (nGlyphId <= (lLocaSize >> 1) - 1) // allow sentinel value to be accessed + if (lLocaSize >= 2 && nGlyphId <= (lLocaSize >> 1) - 1) // allow sentinel value to be accessed { const uint16 * pTable = reinterpret_cast<const uint16 *>(pLoca); return (read(pTable[nGlyphId]) << 1); @@ -1115,7 +1115,7 @@ size_t LocaLookup(gr::gid16 nGlyphId, if (read(pTable->index_to_loc_format) == Sfnt::FontHeader::LongIndexLocFormat) { // loca entries are four bytes - if (nGlyphId <= (lLocaSize >> 2) - 1) + if (lLocaSize >= 4 && nGlyphId <= (lLocaSize >> 2) - 1) { const uint32 * pTable = reinterpret_cast<const uint32 *>(pLoca); return read(pTable[nGlyphId]); --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@openoffice.apache.org For additional commands, e-mail: dev-h...@openoffice.apache.org
