Cff Font Dict operand types - unsigned integers vs. signed integers

[nagyatosz]

I have an open type font with cff table, where the local Subrs of the CFF table is written first to the file, and the Private DICT data is written just after that. It means that the offset in the Private DICT data to the local Subrs is negative value (because the Subrs is written before private Dict data, and the offset to the local Subrs in the case of Private DICT data is relative to the beginning of the Private DICT data). This "problem" seems to cause to fail to sanitize those files.

The CFF specification states that the operands of the DICT data are of type number (integer or real values):

It means the number values can have negative values. But in cff.cc there is (Line 44):

typedef std::pair<uint32_t, DICT_OPERAND_TYPE> Operand;

i.e. the operands are stored as unsigned integers.

Because of this when parsing numbers the values are cast to unsigned integers (lines 281, 292,...). Which seems to be not right.

This causes that the mentioned file fails to sanitize. After I fixed the cff.cc locally the files sanitized successfully.

I will try to create a PR so you can have a look of my suggested changes.

[nagyatosz]

AdobeGaramondItalicSubset.zip
The mentioned problematic .otf file.

[khaledhosny]

This might be technically valid, but I’m not comfortable allowing something like this. CFF table is a notorious source of security bugs and I wouldn’t rule out some implementation somewhere not being well prepared for handling -ve offsets.

Case in point, I tried the attached font with FontForge and hb-view. FontForge shows glyphs either empty or corrupt, hb-view renders shows empty glyphs unless --font-funcs=ft is used. Did you try the font on Windows or macOS? I couldn’t even get it to install on macOS.

[behdad]

Humm. I think this might be easily fixable in HarfBuzz. But I'm also uncomfortable here.

[nagyatosz]

I will share my findings with you:
Yes, the attached file (AdobeGaramondItalicSubset.zip) is very controversial - I will name this font font3B in my comment. The negative offset is written by 3 bytes (b0 range is 28 from the table above).
I have the version of the same font file, the only difference is that the offset is written by 5 bytes (b0 range is 29 from the table above) - I will name this font file font5B in the following.

WINDOWS: font3B does not load, but font5B loads on windows. I checked Font-Validator (https://github.com/HinTak/Font-Validator) first with that file. And I found similar "bug" as here. Just Font-Validator do not parse negative number correctly if the offset (negative number) is written in 3 bytes. They parse it correctly when the offset is written in 5 bytes. It seems to be more like a bug in Font-Validator code, there is missing just one simple cast to get the value correctly (I did this locally), after which the Font-Validator validated the font3B. Otherwise the numbers are not restricted to unsigned values as here in ots.

To sum it up:
font3B does not load on Windows, and it does not validate in Font-Validator, but font-validator validates it after my local bug fix.
font5B loads on windows, and validates in Font-Validator. Windows uses Font-Validator or some of its code? I do not know, Font-Validator was initially developed by MS...

LINUX: Interestingly enough it is the opposite on Linux:
font3B loads successfully on my Linux,
font5B fails to load

fontForge: both font3B, and font5B behave similarly, the font is loaded but the glyphs are corrupted.
OTMaster: both fonts load successfully, the glyphs are correctly decoded.

I am not a security expert in any way, so my opinion may be irrelevant, but I share my thoughts. I am not sure that parsing numbers differently than stated in the specification will help to prevent security issues.
If there is a negative offset in a font like in font3B/font5B, ots will parse it incorrectly, and it will try to load some data at different location than the font designers wanted to load. IMHO it sounds more like a security issue:
So lets stay with font3B: there is -3337 as the offset to local Subrs in that file. ots parses and gets 62199 instead of -3337. Lets imagine font3B has 2 local Subrs, one with some security issues at -3337 and one at 62199 without security issues. After validating with ots, validation will succeed, because ots loads the local Subrs at 62199 and it is the one without security issues. But when the same font is loaded in a program which parses numbers accordingly to the specification, it will load the local Subrs from the location at -3337, which has security issues, while ots validated the font file!

I am not sure what is the problem with font3B or font5B (whether the only problem is with negative offset or there is something else also). I was able to debug two libraries for validating the fonts, and in both cases I found very similar bug. After my local fix the font3B and font5B were validated with both libraries (ots, Font-Validator).

Please let me know your thoughts, I can change the PR, or create a new one with your suggestions, or just leave it up to you to fix it, or leave it as is if you decide so.

I can attach font5B if you would like to have a look.

[nagyatosz]

I forgot to add:
showTTF from fontforge seems to read the values also correctly - it reads -3337, and it seems to decode the local Subrs also.

[rsheeter]

IIUC that means negatives don't consistently work on Linux or Windows. What about Mac?

[nagyatosz]

I do not know about Mac, I will try to figure it out. But I guess it will be the same, that it will depend on the software used, as it is on Windows or Linux. Some software handles it well, some doesn't, some partly... :)

[khaledhosny]

I am not a security expert in any way, so my opinion may be irrelevant, but I share my thoughts. I am not sure that parsing numbers differently than stated in the specification will help to prevent security issues.

We should correctly parse unsigned numbers, that is not controversial. But I don’t think we want to allow negative offsets regardless.

I am not sure what is the problem with font3B or font5B (whether the only problem is with negative offset or there is something else also). I was able to debug two libraries for validating the fonts, and in both cases I found very similar bug. After my local fix the font3B and font5B were validated with both libraries (ots, Font-Validator).

What is the point in making fonts pass validators if they are usable for most implementations. Given the track record of security issues in parsing CFF table, I wouldn’t like to pass fonts that do things most implementations get wrong, which suggests they are completely untested.

[behdad]

@jfkthame any opinion here?

[jfkthame]

I'm unsure what to think about this. Clearly, we should make sure (signed) numbers are parsed correctly; but whether to allow negative offsets (even though some renderers may not handle them) or reject them is trickier.

@nagyatosz what is the source of the original "problematic" font discussed here? I see it's a subset of an Adobe Garamond, but how was it generated? I'm wondering if the original Adobe Garamond Italic had the same issue of negative offsets, or is this a quirk of a particular subsetting tool/workflow?

[nagyatosz]

@jfkthame , the font files were generated by a library for handling .pdf files (by TallComponents) in a development phase. The font generation is already fixed there (no negative offset are generated there any more), after I figured out what is the problem with the generated font files with your tool (ots).

@khaledhosny , thanks for the explanation, I just misunderstood you. I now understand, that you want to correct the parsing of negative numbers, just you want to block negative offsets. Which sounds reasonable, maybe debatable.

[behdad]

LINUX: Interestingly enough it is the opposite on Linux:
font3B loads successfully on my Linux,
font5B fails to load

Can you please attach these two different flavors? I want to fix HarfBuzz at least.

[behdad]

HarfBuzz patch in harfbuzz/harfbuzz#4515

[nagyatosz]

Fonts3B_5B.zip

[behdad]

Fonts3B_5B.zip

Thanks. Both work on HarfBuzz with the PR I submitted.