GHSA-g27h-8qhm-6pff
Excelize: Panic in cellInRange on a worksheet with an empty mergeCell ref
Packages
| Ecosystem | Package | Affected | Fixed in |
|---|---|---|---|
| Go | github.com/xuri/excelize/v2 | >= 2.7.1, < 2.11.1-0.20260820023833-99903a3240e5 | 2.11.1-0.20260820023833-99903a3240e5 |
Details
As written in the advisory (Markdown source, shown as plain text).
### Summary
A worksheet containing `<mergeCell ref=""/>` makes `mergeCellsParser` leave the cached rectangle empty, and `cellInRange` then indexes that empty slice without a length check. Every non-streaming cell API panics with `index out of range [0] with length 0` on the first cell read after opening the file.
### Where it is
`cell.go`, `cellInRange`
func cellInRange(cell, ref []int) bool {
return cell[0] >= ref[0] && cell[0] <= ref[2] && cell[1] >= ref[1] && cell[1] <= ref[3]
}
`ref` is indexed at four positions with no bounds check.
The caller that can hand it an empty slice, `mergeCellsParser`
if ref := ws.MergeCells.Cells[i].Ref; len(ws.MergeCells.Cells[i].rect) == 0 && ref != "" {
if strings.Count(ref, ":") != 1 {
ref += ":" + ref
}
rect, err := rangeRefToCoordinates(ref)
if err != nil {
return cell, err
}
_ = sortCoordinates(rect)
ws.MergeCells.Cells[i].rect = rect
}
if cellInRange([]int{col, row}, ws.MergeCells.Cells[i].rect) {
The `ref != ""` condition means an empty `ref` skips the block that populates `rect`, so `rect` stays nil. The `cellInRange` call on the next line is unconditional and receives that nil slice.
`Ref` comes straight from `xl/worksheets/sheetN.xml` through `encoding/xml`, so an empty string is entirely attacker-controlled.
### Impact
Any consumer that opens an untrusted workbook and reads a cell panics. The loop scans every merged cell, so any cell reference triggers it, not a specific one. Affected entry points include `GetCellValue`, `GetCellType`, `GetCellFormula`, `SetCellValue` and the in-cell branch of `GetPictures`. The streaming `Rows` and `GetRows` use the SAX path and do not go through this parser, and `GetMergeCells` routes through `Rect()` which errors cleanly, which is probably why this has not surfaced before.
There is no option or flag involved; opening the file succeeds and the panic fires on the first cell read. Unless the caller wraps the call in `recover()` it takes the process down.
This is a regression. Commit a34c81e (PR #1500, 2023-03-20) replaced `checkCellInRangeRef`, whose `len(rng) != 2` guard returned cleanly for an empty ref, with the cached-rect fast path above, and the guard did not come along. `git merge-base --is-ancestor` confirms that commit is an ancestor of v2.11.0, so released versions are affected as well as HEAD.
### Proof of concept
Executed at HEAD.
Build a minimal xlsx whose `xl/worksheets/sheet1.xml` contains:
<mergeCells count="1"><mergeCell ref=""></mergeCell></mergeCells>
Then:
f, err := excelize.OpenReader(bytes.NewReader(data))
if err != nil { t.Fatal(err) }
_, _ = f.GetCellValue("Sheet1", "A1")
Observed, running against the repository at HEAD:
panic: runtime error: index out of range [0] with length 0
excelize.cellInRange cell.go:1691
excelize.(*xlsxWorksheet).mergeCellsParser cell.go:1660
excelize.(*File).getCellStringFunc cell.go:1512
excelize.(*File).GetCellValue cell.go:72
`OpenReader` itself returns no error; the file is 1656 bytes. A1, B1 and A2 all reproduce it.
For context on how targeted this is, I ran a battery of 38 crafted files covering data validation, conditional formatting, cell and row references, number formats, cols, hyperlinks, dimension, shared strings and tables. Only the empty-ref merge cell panicked; everything else returned a clean error. The other parsing paths look well guarded.
### Suggested fix
Skip the entry when the rectangle is empty, before the range test:
if len(ws.MergeCells.Cells[i].rect) == 0 {
continue
}
Credit goes to arpitjain099.
References
External links from the advisory. They lead to third-party sites.