CFF fonts that use "random" in their charstrings fail OTS testing

[Pomax]

I have a simple test font that passes OTS testing:

data:font/opentype;base64,T1RUTwAKAIAAAwAgQ0ZGIHqkNIYAAAQEAAABeEdTVUIgpSV2AAAFfAAAAGJPUy8yMNolAgAAARAAAABgY21hcAGPAlsAAAN4AAAAamhlYWRhz0O5AAAArAAAADZoaGVhBr4BgAAAAOQAAAAkaG10eAK8AAAAAAXgAAAAIG1heHAACFAAAAABCAAAAAZuYW1lAa1a/wAAAXAAAAIHcG9zdAADAAEAAAPkAAAAIAABAAAAAQAAKh6V/l8PPPUAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAACvAK8AAAACAACAAAAAAAAAAEAAAQA/rwAAAK8AAAAAAK8AAAAAAAAAAAAAAAAAAAAAAAIAABQAAAIAAAAAwAAAZAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAID0pIABAAGMAfgK8AAABRAQAAUQAAAABAAAAAAAAAAAAAAB+AAYAAAAUAPYAAQAAAAAAAAAMAAAAAQAAAAAAAQAGACQAAQAAAAAAAgAHADYAAQAAAAAAAwABAEsAAQAAAAAABAARAE4AAQAAAAAABQALAIEAAQAAAAAABgAKAKIAAQAAAAAABwAOAMAAAQAAAAAADQAMAOoAAQAAAAAAEwABAQ4AAwABBAkAAAAYAAwAAwABBAkAAQAMACoAAwABBAkAAgAOAD0AAwABBAkAAwACAEwAAwABBAkABAAiAF8AAwABBAkABQAWAIwAAwABBAkABgAUAKwAAwABBAkABwAcAM4AAwABBAkADQAYAPYAAwABBAkAEwACAQ9MaWNlbnNlLWZyZWUATABpAGMAZQBuAHMAZQAtAGYAcgBlAGVDdXN0b20AQwB1AHMAdABvAG1SZWd1bGFyAFIAZQBnAHUAbABhAHItAC1DdXN0b20gR2x5cGggRm9udABDAHUAcwB0AG8AbQAgAEcAbAB5AHAAaAAgAEYAbwBuAHRWZXJzaW9uIDEuMABWAGUAcgBzAGkAbwBuACAAMQAuADBjdXN0b21mb250AGMAdQBzAHQAbwBtAGYAbwBuAHRUcmFkZW1hcmstZnJlZQBUAHIAYQBkAGUAbQBhAHIAawAtAGYAcgBlAGVMaWNlbnNlLWZyZWUATABpAGMAZQBuAHMAZQAtAGYAcgBlAGV+AH4AAAAAAQADAAEAAAAMAAQAXgAAABAAEAADAAAAYwBtAG8AcwB0AHUAfv//AAAAYwBtAG8AcwB0AHUAfv///57/lf+U/5H/kf+R/4kAAQAAAAAAAAAAAAAAAAAAAAAAAQACAAMABAAFAAYABwAAAAMAAAAAAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAABAAQBAAEBAQtjdXN0b21mb250AAEBASP4GwD4HAL4HQP4GQSMDYuL+VD5UAX3qg/3uRD3whGb9/wSAAoBAQwdIyQlJicoKSpWZXJzaW9uIDEuMEN1c3RvbSBHbHlwaCBGb250Q3VzdG9tY21vc3R1fgADAQECDJ8L/wABADkMCiAdC48MFI4MFI0MFIwMFIsMFIsMFSAdkAwUiwwVIR2RDBSODBWMDBUMC5IMFI8MFY0MFQwLkwwUkgwVkQwVDBiUDBSTDBWRDBUMGJUMFI4MFZQMFZUMFQwODAoMCpYMFJIMFZEMFQwYlwwUkwwVkAwVDBiYDBSNDBWXDBWYDBUMCgwKmQwUDBWLDBWMDBWNDBWYDBWZCwABigGLAYwBjQGOAY8BkAAHAQIDBAUGBwAIAQECAwQFBgcILw4ODg4ODg6LixWL+VAF+VCLBYv9UAX9UIsF7+8V+IiLBYv4iAX8iIsFi/yIBQ6LiwaLiwiVCpUL+VAU+VAVAAEAAAAKACQAMgACREZMVAAObGF0bgAOAAQAAAAA//8AAQAAAAFsaWdhAAgAAAABAAAAAQAEAAQAAAABAAgAAQAIAAEAEgACAAEAAQABAAAAAQAEAAcABgAGAAQABQADAAIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACvAAA

This font has a charstring for the character ~ that is really simple, and works fine:

   0    0 rmoveto
   0  700 rlineto
 700    0 rlineto
   0 -700 rlineto
-700    0 rlineto
 100  100 rmoveto
 500    0 rlineto
   0  500 rlineto
-500    0 rlineto
   0 -500 rlineto
endchar

However, if I prepend that charstring with byte code for the instructing 100, 1, put, which puts "100" on the stack, then pops it off and caches it on the transient stack instead, then tx says there is nothing wrong with this font:

data:font/opentype;base64,T1RUTwAKAIAAAwAgQ0ZGIJOXwJMAAAQEAAABfEdTVUIgpSV2AAAFgAAAAGJPUy8yMNolAgAAARAAAABgY21hcAGPAlsAAAN4AAAAamhlYWRhz0O5AAAArAAAADZoaGVhBr4BgAAAAOQAAAAkaG10eAK8AAAAAAXkAAAAIG1heHAACFAAAAABCAAAAAZuYW1lAa1a/wAAAXAAAAIHcG9zdAADAAEAAAPkAAAAIAABAAAAAQAA+Dd92F8PPPUAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAACvAK8AAAACAACAAAAAAAAAAEAAAQA/rwAAAK8AAAAAAK8AAAAAAAAAAAAAAAAAAAAAAAIAABQAAAIAAAAAwAAAZAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAID0pIABAAGMAfgK8AAABRAQAAUQAAAABAAAAAAAAAAAAAAB+AAYAAAAUAPYAAQAAAAAAAAAMAAAAAQAAAAAAAQAGACQAAQAAAAAAAgAHADYAAQAAAAAAAwABAEsAAQAAAAAABAARAE4AAQAAAAAABQALAIEAAQAAAAAABgAKAKIAAQAAAAAABwAOAMAAAQAAAAAADQAMAOoAAQAAAAAAEwABAQ4AAwABBAkAAAAYAAwAAwABBAkAAQAMACoAAwABBAkAAgAOAD0AAwABBAkAAwACAEwAAwABBAkABAAiAF8AAwABBAkABQAWAIwAAwABBAkABgAUAKwAAwABBAkABwAcAM4AAwABBAkADQAYAPYAAwABBAkAEwACAQ9MaWNlbnNlLWZyZWUATABpAGMAZQBuAHMAZQAtAGYAcgBlAGVDdXN0b20AQwB1AHMAdABvAG1SZWd1bGFyAFIAZQBnAHUAbABhAHItAC1DdXN0b20gR2x5cGggRm9udABDAHUAcwB0AG8AbQAgAEcAbAB5AHAAaAAgAEYAbwBuAHRWZXJzaW9uIDEuMABWAGUAcgBzAGkAbwBuACAAMQAuADBjdXN0b21mb250AGMAdQBzAHQAbwBtAGYAbwBuAHRUcmFkZW1hcmstZnJlZQBUAHIAYQBkAGUAbQBhAHIAawAtAGYAcgBlAGVMaWNlbnNlLWZyZWUATABpAGMAZQBuAHMAZQAtAGYAcgBlAGV+AH4AAAAAAQADAAEAAAAMAAQAXgAAABAAEAADAAAAYwBtAG8AcwB0AHUAfv//AAAAYwBtAG8AcwB0AHUAfv///57/lf+U/5H/kf+R/4kAAQAAAAAAAAAAAAAAAAAAAAAAAQACAAMABAAFAAYABwAAAAMAAAAAAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAABAAQBAAEBAQtjdXN0b21mb250AAEBASP4GwD4HAL4HQP4GQSMDYuL+VD5UAX3qg/3uRD3whGb+AASAAoBAQwdIyQlJicoKSpWZXJzaW9uIDEuMEN1c3RvbSBHbHlwaCBGb250Q3VzdG9tY21vc3R1fgADAQECDJ8L/wABADkMCiAdC48MFI4MFI0MFIwMFIsMFIsMFSAdkAwUiwwVIR2RDBSODBWMDBUMC5IMFI8MFY0MFQwLkwwUkgwVkQwVDBiUDBSTDBWRDBUMGJUMFI4MFZQMFZUMFQwODAoMCpYMFJIMFZEMFQwYlwwUkwwVkAwVDBiYDBSNDBWXDBWYDBUMCgwKmQwUDBWLDBWMDBWNDBWYDBWZCwABigGLAYwBjQGOAY8BkAAHAQIDBAUGBwAIAQECAwQFBgcIMw4ODg4ODg7vjAwUi4sVi/lQBflQiwWL/VAF/VCLBe/vFfiIiwWL+IgF/IiLBYv8iAUOi4sGi4sIlQqVC/lQFPlQFQABAAAACgAkADIAAkRGTFQADmxhdG4ADgAEAAAAAP//AAEAAAABbGlnYQAIAAAAAQAAAAEABAAEAAAAAQAIAAEACAABABIAAgABAAEAAQAAAAEABAAHAAYABgAEAAUAAwACAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAArwAAA==

But Firefox and Chrome both die on the resulting font, strongly suggesting the put (and presumably get as well) operation causes OTS to fail a font that is perfectly fine.

[yusukes]

src/cff_type2_charstring.cc does not support all operators, and 'random', 'get', and 'put' are not supported, unfortunately: https://github.com/khaledhosny/ots/blob/master/src/cff_type2_charstring.cc#L604 https://github.com/khaledhosny/ots/blob/master/src/cff_type2_charstring.cc#L589

[Pomax]

Nicely found! Let's take this issue as the starting point to fix that; OTS should not fail in these cases; it'll fail too many cool fonts =)

There's a code comment about index checks for get/put, but that part's actually pretty simple: the transient stack is only ever allowed to be of size 32 (see Adobe Tech Note 5177, appendix B) so any index operand less than 0 or greater than 31 is illegal and should cause a failure. Values between 0 and 31 inclusive are legal.

[behdad]

The Adobe cff engine in FreeType doesn't implement the arithmetic and random operators, so won't be quite useful. But I agree that OTS should NOT reject a font because of a bad charstring even.

[Pomax]

@behdad sounds like Freetype needs a bug filed too, then. There's a lot of amazing things that can be done with proper access to the arithmetic and transient stack operators (nicely randomised geometric transforms, paired with substitutions similar to FF Chartwell, for instance)

[behdad]

@behdad sounds like Freetype needs a bug filed too, then. There's a lot of amazing things that can be done with proper access to the arithmetic and transient stack operators (nicely randomised geometric transforms, paired with substitutions similar to FF Chartwell, for instance)

@Pomax I definitely know the possibilities ;). But none of it will work, because most software is built assuming that a glyph shape won't change. So you won't get, for example, many variations of the same glyph working.

[yusukes]

But I agree that OTS should NOT reject a font because of a bad charstring even.

Just FYI, the charstring check was added primarily for working around these issues (Windows kernel exploit via font-face): https://code.google.com/p/chromium/issues/detail?id=51070 https://code.google.com/p/chromium/issues/detail?id=53142

[Pomax]

@behdad but that's their problem, not Freetype's. I'm a big fan of fixing a chain of broken support one step at a time, eventually we'll hit the browser and everyone can enjoy improved font support =)

Doing charstring checks to determine font validity makes perfect sense, but the chromium issues don't seem to warrant simply failing. For the 51070 issue, it is very easy to detect an empty subrioutine INDEX (since it will list "00 00" as number of routines, and that's the entire INDEX struct); that makes detecting a gsubr call or subr call in a charstring without having subroutines available to call fairly simple: track the availability of subroutines by the number of subroutines available (which should already be the case) and fail any charstring that jumps to a subroutine with a number higher than the subroutine INDEX says are available. Failing a font that tries to jump to a subroutine with an index that is too high makes perfect sense (tx will very much fail any font that does this, too).

For 53142 the solution's even simper. The list of legal operators is defined in Tech Note 5177, including opcodes that are reserved and should not be used, so rejecting reserved operators is literally the correct way to go (although that technically has no relation to this issue, as exploits based on charstrings with illegal op codes do not affect, and are unaffected by, improving support for charstrings without illegal opcodes)

[Pomax]

I'm currently using https://github.com/Pomax/simple-cff-builder (with the charstring fiddling quite easily done here) to generate small test fonts, we could use that to generate a swathe of .otf fonts with various charstring forms that can be used to reference-test OTS.

[khaledhosny]

It would be great if someone can come up with checks that can reject the malicious fonts in the issue linked above while passing the non-malicious ones, otherwise it does not look like such a big priority. Tests are a big plus as always.

[Pomax]

I can prepare a few tiny OT/CFF fonts that try to push/get from outside the transient stack limit (which should definitely be rejected), and fonts that use "reserved" opcodes (which should be rejected too) although my C++ is super duper rusty, so I probably won't be able to write the tests to go with them.

[HinTak]

Okay, it looks like you really want to do an independent check, away from the current font engine while having the current font engine in the same process, so my comments in the last part of https://github.com/khaledhosny/ots/issues/97 is not relevant.

OTOH, it may be be relevant if you hook in AFDKO's cff parser, with extras...

[khaledhosny]

I don’t think I’m ever going to do this, but patches are always welcomed.