GHSA-5h23-36rv-pm65: Input Validation

Summary

File.GetStyle indexes the fill, border and font tables with values taken straight out of xl/styles.xml, and the conditions gating those lookups check only the upper bound. A workbook whose cellXfs entry carries fillId="-1", borderId="-1" or fontId="-1" reaches a negative slice index and panics. excelize has no recover(), so the panic leaves GetStyle and takes the calling process with it.

Same defect class as GHSA-48hm-4h8j-58fg, the negative shared-string index, in a different file. That one was fixed by adding the missing lower bound; these three sites still lack it.

Where it is

styles.go, in GetStyle, lines 1683, 1686 and 1689:

go xf := s.CellXfs.Xf[idx] if extractStyleCondFuncs"fill" { f.extractFills(s.Fills.Fill[xf.FillID], s, style) } if extractStyleCondFuncs"border" { f.extractBorders(s.Borders.Border[xf.BorderID], s, style) } if extractStyleCondFuncs"font" { style.Font = extractFont(s.Fonts.Font[xf.FontID]) }

The conditions, at lines 1171 to 1185, bound only the top:

go "fill": func(xf xlsxXf, s xlsxStyleSheet) bool { return (xf.ApplyFill == nil || (xf.ApplyFill != nil && xf.ApplyFill)) && xf.FillID != nil && s.Fills != nil && xf.FillID < len(s.Fills.Fill) },

xf.FillID < len(...) is satisfied by any negative value. FillID, BorderID and FontID are int unmarshalled directly from the fillId, borderId and fontId attributes, so the value is whatever the file says. The border and font conditions have the same shape.

The correct pattern is already in the same function, six lines above at 1677:

go if idx < 0 || s.CellXfs == nil || len(s.CellXfs.Xf) <= idx { return style, newInvalidStyleID(idx) }

and again at 1783. So the style index itself is guarded on both sides while the three ids it leads to are not.

Proof of concept

Executed against master at f98df08, which is the merge of #2366, so this is current rather than historical. The harness builds a normal styled workbook with excelize, rewrites one attribute of the cellXfs entry inside the zip to -1, reopens it and calls GetCellStyle then GetStyle:

control (all >= 0) ok style=true err=<nil> fillId=-1 PANIC: runtime error: index out of range [-1] borderId=-1 PANIC: runtime error: index out of range [-1] fontId=-1 PANIC: runtime error: index out of range [-1]

The control confirms the harness reads a valid file correctly, so the three panics are the negative ids rather than a broken fixture. Worth mentioning because my first attempt at this harness patched only fillId and appeared to show the other two were fine; they are not, the replacement had simply missed them.

Impact

Any application that opens an untrusted .xlsx and reads cell styling crashes. GetStyle is reached through GetCellStyle and from the style-copying and rendering paths, so it sits on the ordinary read path. With no recover() anywhere in excelize, a CLI or worker exits and a service returns 500 per request unless the caller installed its own recovery middleware.

No memory corruption and no information disclosure. Availability only.

Suggested fix

Add the lower bound to each of the three conditions, matching what line 1677 already does for the style index:

go xf.FillID >= 0 && xf.FillID < len(s.Fills.Fill)

and the same for BorderID and FontID. Three one-line changes in extractStyleCondFuncs.