Copilot commented on code in PR #3852:
URL: https://github.com/apache/avro/pull/3852#discussion_r3564408475
##########
lang/js/lib/files.js:
##########
@@ -187,12 +208,40 @@ BlockDecoder.prototype._decodeHeader = function () {
}
var codec = (header.meta['avro.codec'] || 'null').toString();
- this._decompress = (this._codecs || BlockDecoder.getDefaultCodecs())[codec];
- if (!this._decompress) {
+ var decompress = (this._codecs || BlockDecoder.getDefaultCodecs())[codec];
+ if (!decompress) {
this.emit('error', new Error(f('unknown codec: %s', codec)));
return;
}
+ // Bound the decompressed size of each block to guard against a block with a
+ // very high compression ratio (or a malformed block) expanding to far more
+ // memory than its compressed size. For the built-in deflate codec the limit
+ // is passed to zlib so the allocation itself is capped; every codec's output
+ // is additionally checked as a safeguard.
+ var maxLength = this._maxDecompressLength;
+ if (!this._codecs && codec === 'deflate') {
+ decompress = function (buf, cb) {
+ zlib.inflateRaw(buf, {maxOutputLength: maxLength}, cb);
+ };
+ }
Review Comment:
The zlib `maxOutputLength` cap is only applied when `opts.codecs` is not
provided. If a caller supplies a custom codecs map that includes `deflate:
zlib.inflateRaw` (common when extending the default codecs), the allocation cap
is silently disabled and only the post-decompress length check remains (too
late to prevent decompression-bomb allocations). Consider applying the zlib cap
whenever the selected decompressor is `zlib.inflateRaw`, regardless of whether
it came from the default map or user-supplied codecs.
##########
lang/js/lib/files.js:
##########
@@ -187,12 +208,40 @@ BlockDecoder.prototype._decodeHeader = function () {
}
var codec = (header.meta['avro.codec'] || 'null').toString();
- this._decompress = (this._codecs || BlockDecoder.getDefaultCodecs())[codec];
- if (!this._decompress) {
+ var decompress = (this._codecs || BlockDecoder.getDefaultCodecs())[codec];
+ if (!decompress) {
this.emit('error', new Error(f('unknown codec: %s', codec)));
return;
}
+ // Bound the decompressed size of each block to guard against a block with a
+ // very high compression ratio (or a malformed block) expanding to far more
+ // memory than its compressed size. For the built-in deflate codec the limit
+ // is passed to zlib so the allocation itself is capped; every codec's output
+ // is additionally checked as a safeguard.
+ var maxLength = this._maxDecompressLength;
+ if (!this._codecs && codec === 'deflate') {
+ decompress = function (buf, cb) {
+ zlib.inflateRaw(buf, {maxOutputLength: maxLength}, cb);
+ };
+ }
+ this._decompress = function (buf, cb) {
+ decompress(buf, function (err, data) {
+ if (err) {
+ cb(err);
+ return;
+ }
+ if (data && data.length > maxLength) {
+ cb(new Error(f(
+ 'decompressed block size exceeds the maximum allowed of %d bytes',
+ maxLength
+ )));
+ return;
+ }
+ cb(null, data);
+ });
Review Comment:
When the new max-length safeguard triggers (or any decompressor returns an
error), the error is surfaced via `_createBlockCallback` which currently only
`emit('error', err)` and returns. That leaves `_pending` decremented only on
the success path and does not stop further decoding, which can cause the stream
to hang in a pending state and potentially emit multiple errors for subsequent
blocks. Consider updating the error path to decrement `_pending` and
`destroy(err)` (or otherwise stop processing) on the first decompression/size
error.
--
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]
