iemejia commented on code in PR #3866:
URL: https://github.com/apache/avro/pull/3866#discussion_r3567367898
##########
lang/js/lib/schemas.js:
##########
@@ -1162,12 +1193,27 @@ MapType.prototype._read = function (tap) {
MapType.prototype._skip = function (tap) {
var values = this._values;
+ var minBytes = 1 + getMinBytes(values); // Each entry carries a >=1-byte key.
+ var total = 0;
var len, n;
while ((n = tap.readLong())) {
if (n < 0) {
+ // Sized block: bound the item count too, so the cap cannot be bypassed
by
+ // encoding the block with a negative (byte-sized) count.
+ n = -n;
len = tap.readLong();
+ if (len < 0) {
+ // A negative byte-size would move the read position backwards, which
+ // Tap.isValid() (pos <= buf.length) would not catch, bypassing
+ // truncation detection.
+ throw new Error('negative collection block size');
+ }
+ total += n;
+ checkCollectionBlock(tap, n, minBytes, total);
tap.pos += len;
Review Comment:
Fixed in MapType._skip — the sized-block path now rejects a `len` that
exceeds the remaining buffer, and (when minBytes > 0) a `len` too small to hold
`n` entries at their minimum on-wire size, so the skip can't misalign
subsequent decoding.
##########
lang/js/lib/schemas.js:
##########
@@ -1303,14 +1357,30 @@ ArrayType.prototype._read = function (tap) {
};
ArrayType.prototype._skip = function (tap) {
+ var items = this._items;
+ var minBytes = getMinBytes(items);
+ var total = 0;
var len, n;
while ((n = tap.readLong())) {
if (n < 0) {
+ // Sized block: bound the item count too, so the cap cannot be bypassed
by
+ // encoding the block with a negative (byte-sized) count.
+ n = -n;
len = tap.readLong();
+ if (len < 0) {
+ // A negative byte-size would move the read position backwards, which
+ // Tap.isValid() (pos <= buf.length) would not catch, bypassing
+ // truncation detection.
+ throw new Error('negative collection block size');
+ }
+ total += n;
+ checkCollectionBlock(tap, n, minBytes, total);
tap.pos += len;
Review Comment:
Fixed the same way in ArrayType._skip — added the `len <= remaining` and `n
<= len / minBytes` sanity checks before `tap.pos += len`.
--
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]
