Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/image/font/sfnt: horizontal metrics table (hmtx) validation is incorrect #28379

Closed
dennwc opened this issue Oct 25, 2018 · 4 comments
Closed

x/image/font/sfnt: horizontal metrics table (hmtx) validation is incorrect #28379

dennwc opened this issue Oct 25, 2018 · 4 comments
Milestone

Comments

@dennwc
Copy link
Contributor

@dennwc dennwc commented Oct 25, 2018

What did you do?

Parse an embedded TTF font from PDF object.

What did you expect to see?

no error

What did you see instead?

sfnt: invalid hmtx table

Details

It seems that the validation code for hmtx table is incorrect. The library assumes 2*numGlyphs + 2*numHMetrics, while the spec describes it as 4*numHMetrics [+ 2*(numGlyphs-numHMetrics)].

@gopherbot gopherbot added this to the Unreleased milestone Oct 25, 2018
@dennwc dennwc closed this Oct 25, 2018
@dennwc dennwc reopened this Oct 25, 2018
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 25, 2018

Change https://golang.org/cl/144080 mentions this issue: font/sfnt: fix hmtx table size validation

@dennwc
Copy link
Contributor Author

@dennwc dennwc commented Oct 30, 2018

Values in the font: numHMetrics=183, numGlyphs=3415, hmtx.length=732.

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 30, 2018

Change https://golang.org/cl/146081 mentions this issue: font/sfnt: add parsing tests

@nigeltao
Copy link
Contributor

@nigeltao nigeltao commented Oct 31, 2018

In case anyone else was confused, the square brackets in @dennwc's original post: 4*numHMetrics [+ 2*(numGlyphs-numHMetrics)] means optional. In different words, the bug report argues that the sfnt package should accept either of 4*nHM or 4*nHM+2*(nG-nHM). Note that the latter form simplifies to 2*nHM+2*nG, which is what the package implements today.

The original bug report also claimed that the spec allows for just 4*nHM, but I don't think the spec actually says that.

Still, a comment in the freetype library's ttload.c says:

/* Some tables have such a simple structure that clipping its     */
/* contents is harmless.  This also makes FreeType less sensitive */
/* to invalid table lengths (which programs like Acroread seem to */
/* ignore in general).                                            */
if ( table.Tag == TTAG_hmtx || table.Tag == TTAG_vmtx ) etc

and also, eyeballing its tt_face_get_metrics function in ttmtx.c suggests that FreeType doesn't enforce that the hmtx table is exactly 2*nHM + 2*nG long.

@golang golang locked and limited conversation to collaborators Nov 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.