GHSA-g27h-8qhm-6pff: Out-of-bounds Read

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.