GHSA-g27h-8qhm-6pff

Dashboard / Vulnerabilities / GHSA-g27h-8qhm-6pff

GHSA-g27h-8qhm-6pff

Summary: Excelize: Panic in cellInRange on a worksheet with an empty mergeCell ref

Details: ### 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: https://github.com/qax-os/excelize/security/advisories/GHSA-g27h-8qhm-6pff, https://github.com/qax-os/excelize/pull/2379, https://github.com/qax-os/excelize/commit/99903a3240e58a47ff28fb8e05a03bd9d496ec24, https://github.com/qax-os/excelize