Skip to content

Commit

Permalink
font/sfnt: verify the total number of contour points
Browse files Browse the repository at this point in the history
The SFNT file format explicitly lists the number of points in each
simple (non-compound) glyph and, in this package, this is loaded in func
loadGlyf as the numPoints variable. numPoints is then passed to func
findXYIndexes to verify that the (variable length) remaning glyph data
has content for that many points. loadGlyf then uses a glyfIter to
iterate over those points, but prior to this commit, fails to enforce
that the glyfIter also honors numPoints when walking each contour of a
glyph. This can lead to a panic (slice index out of bounds) on a
malformed SFNT file, if glyfIter then tries to walk too many points.

Fixes golang/go#48006

Change-Id: I92530e570eb37ce0087927ca23060acebe0a7705
Reviewed-on: https://go-review.googlesource.com/c/image/+/358994
Reviewed-by: Andrew Gerrand <adg@golang.org>
Trust: Nigel Tao <nigeltao@golang.org>
  • Loading branch information
nigeltao committed Oct 28, 2021
1 parent a66eb64 commit 6944b10
Showing 1 changed file with 10 additions and 4 deletions.
14 changes: 10 additions & 4 deletions font/sfnt/truetype.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,10 @@ func loadGlyf(f *Font, b *Buffer, x GlyphIndex, stackBottom, recursionDepth uint
xIndex: xIndex,
yIndex: yIndex,
endIndex: glyfHeaderLen,
// The -1 is because the contour-end index in the file format is
// inclusive, but Go's slice[:index] semantics are exclusive.
// The -1 on prevEnd and finalEnd are because the contour-end index in
// the file format is inclusive, but Go's slice[:index] is exclusive.
prevEnd: -1,
finalEnd: int32(numPoints - 1),
numContours: int32(numContours),
}
for g.nextContour() {
Expand Down Expand Up @@ -334,9 +335,11 @@ type glyfIter struct {
yIndex int32

// endIndex points to the uint16 that is the inclusive point index of the
// current contour's end. prevEnd is the previous contour's end.
// current contour's end. prevEnd is the previous contour's end. finalEnd
// should match the final contour's end.
endIndex int32
prevEnd int32
finalEnd int32

// c and p count the current contour and point, up to numContours and
// numPoints.
Expand Down Expand Up @@ -386,13 +389,16 @@ type glyfIter struct {

func (g *glyfIter) nextContour() (ok bool) {
if g.c == g.numContours {
if g.prevEnd != g.finalEnd {
g.err = errInvalidGlyphData
}
return false
}
g.c++

end := int32(u16(g.data[g.endIndex:]))
g.endIndex += 2
if end <= g.prevEnd {
if (end <= g.prevEnd) || (g.finalEnd < end) {
g.err = errInvalidGlyphData
return false
}
Expand Down

0 comments on commit 6944b10

Please sign in to comment.