iemejia commented on code in PR #3866:
URL: https://github.com/apache/avro/pull/3866#discussion_r3566894435
##########
lang/js/lib/schemas.js:
##########
@@ -1162,12 +1188,21 @@ 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();
+ total += n;
+ checkCollectionBlock(tap, n, minBytes, total);
tap.pos += len;
Review Comment:
Fixed in b50e676c64: the sized-block skip path now rejects a negative block
byte-size before `tap.pos += len`, so it can't move the position backwards and
bypass truncation detection (Tap.isValid only checks pos <= buf.length).
##########
lang/js/lib/schemas.js:
##########
@@ -1303,14 +1346,24 @@ 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();
+ total += n;
+ checkCollectionBlock(tap, n, minBytes, total);
tap.pos += len;
Review Comment:
Fixed in b50e676c64: same negative-byte-size guard added to the other
collection's _skip sized-block branch.
##########
lang/js/test/test_schemas.js:
##########
@@ -939,6 +939,18 @@ describe('types', function () {
assert.strictEqual(t.getName(), undefined);
});
+ it('rejects a huge block count', function () {
+ var t = new types.MapType({type: 'map', values: 'long'});
+ var buf = Buffer.from([0x80, 0x88, 0xde, 0xbe, 0x01, 0x00]);
+ assert.throws(function () { t.fromBuffer(buf); }, /collection/);
+ });
+
+ it('reads a small map', function () {
+ var t = new types.MapType({type: 'map', values: 'int'});
+ var buf = t.toBuffer({a: 1, b: 2});
+ assert.deepEqual(t.fromBuffer(buf), {a: 1, b: 2});
+ });
Review Comment:
Fixed in b50e676c64: added MapType coverage mirroring the array cases — huge
block count while skipping, huge negative (sized) block count while skipping, a
rejected negative block byte-size, and a small map decoded under schema
resolution.
--
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]
