fix(security): harden reader against malicious xlsx files (0.3.0) - #3
Merged
Merged
Conversation
Replace the regex-based XML walking in the reader with linear indexOf scanners (elements/firstElement/stripElements/attr in xml.ts). Unclosed tags now fail fast with a descriptive error instead of scaling quadratically (ReDoS): 86 KB of unclosed <row> went from ~220 ms to ~1 ms. Bound dense materialization: toRows() and toObjects() accept maxCells (default 20M) and throw a RangeError above it, so a file with a single cell at XFD1048576 no longer exhausts memory. toRows() now walks only populated cells. Add an aggregate decompression budget to unzipSync (maxTotalSize, maxEntrySize), enforce it on the actual inflated output rather than the declared size, verify declared sizes, and expose it as ReadOptions.maxDecompressedSize. Reject XML parts above MAX_PART_SIZE with a descriptive error instead of an opaque V8 string-length failure. Decode XML entities in a single pass so &lt; stays the literal <, and raise a descriptive error for code points outside Unicode instead of a bare RangeError. Build toObjects() rows with Object.fromEntries so a "__proto__" header from the file becomes an own property instead of altering the prototype. Export the new options, limits and types from the package entry point. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsvdZfRkdMH19tw7PyDvRY
Record the findings of the September 2026 review as a tracked backlog: security fixes and open hardening items, confirmed correctness bugs with repro and proposed fixes, engineering/tooling improvements, and a feature roadmap grouped by version. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsvdZfRkdMH19tw7PyDvRY
The linear scanner yields an empty inner string for a self-closing <v /> (openpyxl writes it for formula cells without a cached value), which Number() turned into 0 and a shared-string lookup into index 0. The previous regex simply did not match, so the value was null; restore that behaviour and cover it with the openpyxl shape. Also mark the items already resolved by PR #2 in the docs backlog. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsvdZfRkdMH19tw7PyDvRY
…is-improvements-3dokrb
The "limitation" test from PR #2 documented the pre-fix behaviour where a __proto__ header dropped its column. toObjects() now builds rows with Object.fromEntries, so replace it with an assertion of the fixed behaviour: the key is an own property and the prototype is untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsvdZfRkdMH19tw7PyDvRY
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsvdZfRkdMH19tw7PyDvRY
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resumen
Endurece
read()frente a archivos.xlsxmaliciosos o corruptos, añade una carpetadocs/con el backlog de la revisión y sube la versión a 0.3.0 (hay opciones públicas nuevas).Cada punto se reprodujo con un script antes del arreglo y está cubierto en
test/security.test.ts.Fallos de seguridad corregidos
[\s\S]*?; ante etiquetas sin cerrar el coste era cuadrático (86 KB de<row>sin cerrar: 221 ms; extrapolado, 10 MB ≈ una hora de CPU).src/xml.tsincorpora un escáner lineal (elements,firstElement,stripElements,attrbasado enindexOf). Ahora: 1 ms, y una etiqueta sin cerrar lanzaXML malformado: falta </row>.toRows()/toObjects(). Materializan el rectángulo denso; una celda enXFD1048576exigía 17 000 M de entradas. Nueva opciónmaxCells(predeterminado 20 M) conRangeErrordescriptivo.toRows()recorre solo celdas pobladas (~4× más rápido).unzipSyncaceptamaxTotalSize/maxEntrySize, el tope se aplica sobre la salida real de zlib (no sobre el tamaño declarado, que se verifica), yread()lo expone comoReadOptions.maxDecompressedSize. Las partes XML mayores deMAX_PART_SIZE(256 MiB) fallan con mensaje propio en lugar del error opaco de V8.&lt;es el texto literal<) y error descriptivo para referencias fuera de Unicode en vez de unRangeErrorcrudo.__proto__entoObjects().Object.fromEntriesdefine propiedades propias; el prototipo queda intacto.Otros cambios
<v/>vacío (openpyxl lo escribe en fórmulas sin valor cacheado) se lee comonull, no como0. Verificado contra el fixture real de openpyxl.ReadOptions,InvalidSheetNamesMode,DenseOptions,DEFAULT_MAX_CELLS,MAX_PART_SIZE,MAX_ENTRY_SIZE,MAX_TOTAL_SIZE,MAX_ROWS,MAX_COLS.docs/: backlog interno (seguridad, bugs confirmados, mejoras de tooling, roadmap por versiones). No se publica en npm.maintras PR test(helpers): extract synthetic xlsx builders for reuse #2 y adapta el test de__proto__al comportamiento corregido.chore: bump version to 0.3.0.Verificación
pnpm testpnpm test:coveragepnpm lint,pnpm typecheck,pnpm build🤖 Generated with Claude Code