iemejia opened a new pull request, #3866:
URL: https://github.com/apache/avro/pull/3866

   ## What is the purpose of the change
   
   An `array` or `map` block is encoded as an item count followed by that many 
items. A malicious or truncated input can declare a very large count while 
carrying little or no data, which causes a correspondingly large allocation 
before the shortfall is noticed.
   
   The JavaScript SDK already bounds length-prefixed `bytes`/`string`/`fixed` 
values against its in-memory buffer (`Tap.readFixed`/`readString` check the 
position against the buffer length before allocating), which is why it had no 
available-bytes sub-task under 
[AVRO-4292](https://issues.apache.org/jira/browse/AVRO-4292). Collections, 
however, were **not** bounded: `ArrayType._read` and `MapType._read` read the 
block item count and push elements incrementally into a plain array/object with 
no cap and no bytes-remaining check.
   
   Because zero-byte elements (e.g. `null`) consume no input, a ~6 byte payload 
such as `{"type":"array","items":"null"}` declaring a block count of 
200,000,000 builds a 200M-entry array and exhausts the heap. Reproduced on 
`main`:
   
   ```
   FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed -
   JavaScript heap out of memory
   ```
   
   under `node --max-old-space-size=256`. Even truncated non-zero-byte 
collections are affected, because an out-of-range `Buffer` read returns 
`undefined` rather than throwing, so the loop runs to the full declared count; 
the `tap.isValid()` check only happens *after* `_read` returns.
   
   ### Fix
   
   `ArrayType._read`/`_skip` and `MapType._read`/`_skip` now bound each block, 
consistent with the other SDKs:
   
   - reject a block whose element count could not be backed by the bytes 
remaining, using `getMinBytes()` (the minimum on-wire size of the element 
schema) so a zero-byte element type is never falsely rejected;
   - cap the cumulative count of zero-byte elements (`MAX_COLLECTION_ITEMS`, 
default 10,000,000);
   - apply a structural cap to every collection (`MAX_COLLECTION_STRUCTURAL`, 
default `Integer.MAX_VALUE - 8`).
   
   When set, the `AVRO_MAX_COLLECTION_ITEMS` environment variable caps both 
limits. Under schema resolution the minimum is computed from the **writer** 
element type (what is actually on the wire, precomputed in `_updateResolver`), 
so legitimate collections are not falsely rejected.
   
   This is a sub-task of 
[AVRO-4292](https://issues.apache.org/jira/browse/AVRO-4292) and resolves 
[AVRO-4301](https://issues.apache.org/jira/browse/AVRO-4301).
   
   ## Verifying this change
   
   This change added tests and can be verified as follows:
   
   - Extended `lang/js/test/test_schemas.js` with, for both arrays and maps: a 
huge zero-byte block count rejected, a huge non-zero-byte block count rejected 
(above the remaining bytes), a small collection that still decodes, the skip 
path bounded, and both the rejection and a small legitimate decode under schema 
resolution.
   - Manually verified against the PoC (`array<null>`, block count 200,000,000) 
under `--max-old-space-size=256`: rejected with a clean error instead of an OOM 
crash.
   - Run: `cd lang/js && npm test` (391 passing) and `npm run lint` (clean).
   
   ## Documentation
   
   - Does this pull request introduce a new feature? (no — hardening / 
robustness)
   - If yes, how is the feature documented? (not applicable; adds the 
`AVRO_MAX_COLLECTION_ITEMS` environment override, consistent with the other 
SDKs)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to