Skip to content

Commit 1904533

Browse files
dcodeIOdcodeIO
authored
fix: Reject malformed field tag varints (#2224)
1 parent c87b149 commit 1904533

15 files changed

Lines changed: 355 additions & 291 deletions

File tree

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@ sandbox/
1010
.nyc_output
1111
dist/
1212
.gitpod.yml
13+
.tmp/

‎bench/data/static_pbjs.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/*eslint-disable block-scoped-var, id-length, no-control-regex, no-magic-numbers, no-prototype-builtins, no-redeclare, no-shadow, no-var, sort-vars, jsdoc/require-param*/
1+
/*eslint-disable block-scoped-var, id-length, no-control-regex, no-magic-numbers, no-prototype-builtins, no-redeclare, no-shadow, no-var, sort-vars, default-case, jsdoc/require-param*/
22
"use strict";
33

44
var $protobuf = require("../../minimal");
@@ -48,7 +48,7 @@ $root.Test = (function() {
4848
var end = length === undefined ? reader.len : reader.pos + length, message = _target || new $root.Test(), value;
4949
while (reader.pos < end) {
5050
var start = reader.pos;
51-
var tag = reader.uint32();
51+
var tag = reader.tag();
5252
if (tag === _end) {
5353
_end = undefined;
5454
break;
@@ -136,7 +136,7 @@ $root.Test = (function() {
136136
var end = length === undefined ? reader.len : reader.pos + length, message = _target || new $root.Test.Inner(), value;
137137
while (reader.pos < end) {
138138
var start = reader.pos;
139-
var tag = reader.uint32();
139+
var tag = reader.tag();
140140
if (tag === _end) {
141141
_end = undefined;
142142
break;
@@ -212,7 +212,7 @@ $root.Test = (function() {
212212
var end = length === undefined ? reader.len : reader.pos + length, message = _target || new $root.Test.Inner.InnerInner(), value;
213213
while (reader.pos < end) {
214214
var start = reader.pos;
215-
var tag = reader.uint32();
215+
var tag = reader.tag();
216216
if (tag === _end) {
217217
_end = undefined;
218218
break;
@@ -315,7 +315,7 @@ $root.Outer = (function() {
315315
var end = length === undefined ? reader.len : reader.pos + length, message = _target || new $root.Outer(), value;
316316
while (reader.pos < end) {
317317
var start = reader.pos;
318-
var tag = reader.uint32();
318+
var tag = reader.tag();
319319
if (tag === _end) {
320320
_end = undefined;
321321
break;

‎index.d.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,6 +1191,12 @@ export class Reader {
11911191
*/
11921192
public uint32(): number;
11931193

1194+
/**
1195+
* Reads a field tag.
1196+
* @returns Tag read
1197+
*/
1198+
public tag(): number;
1199+
11941200
/**
11951201
* Reads a varint as a signed 32 bit value.
11961202
* @returns Value read

‎src/decoder.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ function decoder(mtype) {
3535
("var c=l===undefined?r.len:r.pos+l,m=g||new this.ctor" + (hasMapField ? ",k,v" : hasImplicitPresenceField ? ",v" : ""))
3636
("while(r.pos<c){")
3737
("var s=r.pos")
38-
("var t=r.uint32()")
38+
("var t=r.tag()")
3939
("if(t===z){")
4040
("z=undefined")
4141
("break")
@@ -70,7 +70,7 @@ function decoder(mtype) {
7070

7171
gen
7272
("while(r.pos<c2){")
73-
("var t2=r.uint32()")
73+
("var t2=r.tag()")
7474
("u=t2&7")
7575
("switch(t2>>>=3){")
7676
("case 1:")

‎src/reader.js‎

Lines changed: 75 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -98,52 +98,88 @@ Reader.prototype.raw = function read_raw(start, end) {
9898
* @function
9999
* @returns {number} Value read
100100
*/
101-
Reader.prototype.uint32 = (function read_uint32_setup() {
102-
return function read_uint32() {
103-
var buf = this.buf,
104-
pos = this.pos,
105-
value = (buf[pos] & 127) >>> 0;
106-
if (buf[pos++] < 128) {
107-
this.pos = pos;
108-
return value;
109-
}
110-
value = (value | (buf[pos] & 127) << 7) >>> 0;
111-
if (buf[pos++] < 128) {
112-
this.pos = pos;
113-
return value;
114-
}
115-
value = (value | (buf[pos] & 127) << 14) >>> 0;
116-
if (buf[pos++] < 128) {
117-
this.pos = pos;
118-
return value;
119-
}
120-
value = (value | (buf[pos] & 127) << 21) >>> 0;
121-
if (buf[pos++] < 128) {
101+
Reader.prototype.uint32 = function read_uint32() {
102+
var buf = this.buf,
103+
pos = this.pos,
104+
value = (buf[pos] & 127) >>> 0;
105+
if (buf[pos++] < 128) {
106+
this.pos = pos;
107+
return value;
108+
}
109+
value = (value | (buf[pos] & 127) << 7) >>> 0;
110+
if (buf[pos++] < 128) {
111+
this.pos = pos;
112+
return value;
113+
}
114+
value = (value | (buf[pos] & 127) << 14) >>> 0;
115+
if (buf[pos++] < 128) {
116+
this.pos = pos;
117+
return value;
118+
}
119+
value = (value | (buf[pos] & 127) << 21) >>> 0;
120+
if (buf[pos++] < 128) {
121+
this.pos = pos;
122+
return value;
123+
}
124+
value = (value | (buf[pos] & 15) << 28) >>> 0;
125+
if (buf[pos++] < 128) {
126+
this.pos = pos;
127+
return value;
128+
}
129+
130+
for (var i = 0; i < 5; ++i) {
131+
/* istanbul ignore if */
132+
if (pos >= this.len) {
122133
this.pos = pos;
123-
return value;
134+
throw indexOutOfRange(this);
124135
}
125-
value = (value | (buf[pos] & 15) << 28) >>> 0;
126136
if (buf[pos++] < 128) {
127137
this.pos = pos;
128138
return value;
129139
}
140+
}
141+
/* istanbul ignore next */
142+
this.pos = pos;
143+
throw Error("invalid varint encoding");
144+
};
130145

131-
for (var i = 0; i < 5; ++i) {
132-
/* istanbul ignore if */
133-
if (pos >= this.len) {
134-
this.pos = pos;
135-
throw indexOutOfRange(this);
136-
}
137-
if (buf[pos++] < 128) {
138-
this.pos = pos;
139-
return value;
140-
}
141-
}
142-
/* istanbul ignore next */
146+
/**
147+
* Reads a field tag.
148+
* @function
149+
* @returns {number} Tag read
150+
*/
151+
Reader.prototype.tag = function read_tag() {
152+
var buf = this.buf,
153+
pos = this.pos,
154+
value = (buf[pos] & 127) >>> 0;
155+
if (buf[pos++] < 128) {
143156
this.pos = pos;
144-
throw Error("invalid varint encoding");
145-
};
146-
})();
157+
return value;
158+
}
159+
value = (value | (buf[pos] & 127) << 7) >>> 0;
160+
if (buf[pos++] < 128) {
161+
this.pos = pos;
162+
return value;
163+
}
164+
value = (value | (buf[pos] & 127) << 14) >>> 0;
165+
if (buf[pos++] < 128) {
166+
this.pos = pos;
167+
return value;
168+
}
169+
value = (value | (buf[pos] & 127) << 21) >>> 0;
170+
if (buf[pos++] < 128) {
171+
this.pos = pos;
172+
return value;
173+
}
174+
value = (value | (buf[pos] & 15) << 28) >>> 0;
175+
if (buf[pos] < 128 && (buf[pos] & 112) === 0) {
176+
this.pos = pos + 1;
177+
return value;
178+
}
179+
180+
this.pos = pos + 1;
181+
throw Error("invalid tag encoding");
182+
};
147183

148184
/**
149185
* Reads a varint as a signed 32 bit value.
@@ -439,7 +475,7 @@ Reader.prototype.skipType = function(wireType, depth, fieldNumber) {
439475
break;
440476
case 3:
441477
while (true) {
442-
var tag = this.uint32();
478+
var tag = this.tag();
443479
var nestedField = tag >>> 3;
444480
wireType = tag & 7;
445481
if (!nestedField)

‎tests/api_type.js‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,11 @@ tape.test("decode known fields by wire type", function(test) {
205205
var field1Fixed64 = [ 1 << 3 | 1, 1, 2, 3, 4, 5, 6, 7, 8 ];
206206
var field1Group = [ 1 << 3 | 3, 1 << 3 | 4 ];
207207
var field1GroupField2End = [ 1 << 3 | 3, 2 << 3 | 4 ];
208+
var field1TagWithContinuation = 1 << 3 | 128;
209+
var fieldNumberTooHigh = [ field1TagWithContinuation, 128, 128, 128, 128, 15, 210, 9 ];
210+
var fieldNumberSlightlyTooHigh = [ field1TagWithContinuation, 128, 128, 128, 64, 210, 9 ];
211+
var overlongTagVarint = [ field1TagWithContinuation, 128, 128, 128, 128, 128, 128, 128, 0, 210, 9 ];
212+
var tagVarintMoreThanTenBytes = [ field1TagWithContinuation, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 0, 210, 9 ];
208213

209214
test.throws(function() {
210215
Message.decode(protobuf.util.newBuffer(field0Varint));
@@ -227,6 +232,22 @@ tape.test("decode known fields by wire type", function(test) {
227232
test.throws(function() {
228233
Message.decode(protobuf.util.newBuffer(field1GroupField2End));
229234
}, /invalid end group tag/, "should reject mismatched unknown group tags");
235+
236+
test.throws(function() {
237+
Message.decode(protobuf.util.newBuffer(fieldNumberTooHigh));
238+
}, /invalid tag encoding/, "should reject tags above the maximum field number");
239+
240+
test.throws(function() {
241+
Message.decode(protobuf.util.newBuffer(fieldNumberSlightlyTooHigh));
242+
}, /invalid tag encoding/, "should reject tags above uint32 range");
243+
244+
test.throws(function() {
245+
Message.decode(protobuf.util.newBuffer(overlongTagVarint));
246+
}, /invalid tag encoding/, "should reject overlong tag varints");
247+
248+
test.throws(function() {
249+
Message.decode(protobuf.util.newBuffer(tagVarintMoreThanTenBytes));
250+
}, /invalid tag encoding/, "should reject tag varints longer than ten bytes");
230251
test.end();
231252
});
232253

‎tests/data/comments.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/*eslint-disable block-scoped-var, id-length, no-control-regex, no-magic-numbers, no-prototype-builtins, no-redeclare, no-shadow, no-var, sort-vars, jsdoc/require-param*/
1+
/*eslint-disable block-scoped-var, id-length, no-control-regex, no-magic-numbers, no-prototype-builtins, no-redeclare, no-shadow, no-var, sort-vars, default-case, jsdoc/require-param*/
22
"use strict";
33

44
var $protobuf = require("../../minimal");
@@ -134,7 +134,7 @@ $root.Test1 = (function() {
134134
var end = length === undefined ? reader.len : reader.pos + length, message = _target || new $root.Test1(), value;
135135
while (reader.pos < end) {
136136
var start = reader.pos;
137-
var tag = reader.uint32();
137+
var tag = reader.tag();
138138
if (tag === _end) {
139139
_end = undefined;
140140
break;
@@ -394,7 +394,7 @@ $root.Test2 = (function() {
394394
var end = length === undefined ? reader.len : reader.pos + length, message = _target || new $root.Test2();
395395
while (reader.pos < end) {
396396
var start = reader.pos;
397-
var tag = reader.uint32();
397+
var tag = reader.tag();
398398
if (tag === _end) {
399399
_end = undefined;
400400
break;

‎tests/data/convert.js‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/*eslint-disable block-scoped-var, id-length, no-control-regex, no-magic-numbers, no-prototype-builtins, no-redeclare, no-shadow, no-var, sort-vars, jsdoc/require-param*/
1+
/*eslint-disable block-scoped-var, id-length, no-control-regex, no-magic-numbers, no-prototype-builtins, no-redeclare, no-shadow, no-var, sort-vars, default-case, jsdoc/require-param*/
22
"use strict";
33

44
var $protobuf = require("../../minimal");
@@ -213,7 +213,7 @@ $root.Message = (function() {
213213
var end = length === undefined ? reader.len : reader.pos + length, message = _target || new $root.Message(), key, value;
214214
while (reader.pos < end) {
215215
var start = reader.pos;
216-
var tag = reader.uint32();
216+
var tag = reader.tag();
217217
if (tag === _end) {
218218
_end = undefined;
219219
break;
@@ -313,7 +313,7 @@ $root.Message = (function() {
313313
key = "";
314314
value = 0;
315315
while (reader.pos < end2) {
316-
var tag2 = reader.uint32();
316+
var tag2 = reader.tag();
317317
wireType = tag2 & 7;
318318
switch (tag2 >>>= 3) {
319319
case 1:
@@ -458,7 +458,7 @@ $root.Message = (function() {
458458
if (object.stringRepeated) {
459459
if (!Array.isArray(object.stringRepeated))
460460
throw TypeError(".Message.stringRepeated: array expected");
461-
message.stringRepeated = [];
461+
message.stringRepeated = Array(object.stringRepeated.length);
462462
for (var i = 0; i < object.stringRepeated.length; ++i)
463463
message.stringRepeated[i] = String(object.stringRepeated[i]);
464464
}
@@ -475,7 +475,7 @@ $root.Message = (function() {
475475
if (object.uint64Repeated) {
476476
if (!Array.isArray(object.uint64Repeated))
477477
throw TypeError(".Message.uint64Repeated: array expected");
478-
message.uint64Repeated = [];
478+
message.uint64Repeated = Array(object.uint64Repeated.length);
479479
for (var i = 0; i < object.uint64Repeated.length; ++i)
480480
if ($util.Long)
481481
(message.uint64Repeated[i] = $util.Long.fromValue(object.uint64Repeated[i])).unsigned = true;
@@ -495,7 +495,7 @@ $root.Message = (function() {
495495
if (object.bytesRepeated) {
496496
if (!Array.isArray(object.bytesRepeated))
497497
throw TypeError(".Message.bytesRepeated: array expected");
498-
message.bytesRepeated = [];
498+
message.bytesRepeated = Array(object.bytesRepeated.length);
499499
for (var i = 0; i < object.bytesRepeated.length; ++i)
500500
if (typeof object.bytesRepeated[i] === "string")
501501
$util.base64.decode(object.bytesRepeated[i], message.bytesRepeated[i] = $util.newBuffer($util.base64.length(object.bytesRepeated[i])), 0);
@@ -522,7 +522,7 @@ $root.Message = (function() {
522522
if (object.enumRepeated) {
523523
if (!Array.isArray(object.enumRepeated))
524524
throw TypeError(".Message.enumRepeated: array expected");
525-
message.enumRepeated = [];
525+
message.enumRepeated = Array(object.enumRepeated.length);
526526
for (var i = 0; i < object.enumRepeated.length; ++i)
527527
switch (object.enumRepeated[i]) {
528528
default:
@@ -600,7 +600,7 @@ $root.Message = (function() {
600600
if (message.stringVal != null && message.hasOwnProperty("stringVal"))
601601
object.stringVal = message.stringVal;
602602
if (message.stringRepeated && message.stringRepeated.length) {
603-
object.stringRepeated = [];
603+
object.stringRepeated = Array(message.stringRepeated.length);
604604
for (var j = 0; j < message.stringRepeated.length; ++j)
605605
object.stringRepeated[j] = message.stringRepeated[j];
606606
}
@@ -610,7 +610,7 @@ $root.Message = (function() {
610610
else
611611
object.uint64Val = options.longs === String ? $util.Long.prototype.toString.call(message.uint64Val) : options.longs === Number ? new $util.LongBits(message.uint64Val.low >>> 0, message.uint64Val.high >>> 0).toNumber(true) : message.uint64Val;
612612
if (message.uint64Repeated && message.uint64Repeated.length) {
613-
object.uint64Repeated = [];
613+
object.uint64Repeated = Array(message.uint64Repeated.length);
614614
for (var j = 0; j < message.uint64Repeated.length; ++j)
615615
if (typeof message.uint64Repeated[j] === "number")
616616
object.uint64Repeated[j] = options.longs === String ? String(message.uint64Repeated[j]) : message.uint64Repeated[j];
@@ -620,14 +620,14 @@ $root.Message = (function() {
620620
if (message.bytesVal != null && message.hasOwnProperty("bytesVal"))
621621
object.bytesVal = options.bytes === String ? $util.base64.encode(message.bytesVal, 0, message.bytesVal.length) : options.bytes === Array ? Array.prototype.slice.call(message.bytesVal) : message.bytesVal;
622622
if (message.bytesRepeated && message.bytesRepeated.length) {
623-
object.bytesRepeated = [];
623+
object.bytesRepeated = Array(message.bytesRepeated.length);
624624
for (var j = 0; j < message.bytesRepeated.length; ++j)
625625
object.bytesRepeated[j] = options.bytes === String ? $util.base64.encode(message.bytesRepeated[j], 0, message.bytesRepeated[j].length) : options.bytes === Array ? Array.prototype.slice.call(message.bytesRepeated[j]) : message.bytesRepeated[j];
626626
}
627627
if (message.enumVal != null && message.hasOwnProperty("enumVal"))
628628
object.enumVal = options.enums === String ? $root.Message.SomeEnum[message.enumVal] === undefined ? message.enumVal : $root.Message.SomeEnum[message.enumVal] : message.enumVal;
629629
if (message.enumRepeated && message.enumRepeated.length) {
630-
object.enumRepeated = [];
630+
object.enumRepeated = Array(message.enumRepeated.length);
631631
for (var j = 0; j < message.enumRepeated.length; ++j)
632632
object.enumRepeated[j] = options.enums === String ? $root.Message.SomeEnum[message.enumRepeated[j]] === undefined ? message.enumRepeated[j] : $root.Message.SomeEnum[message.enumRepeated[j]] : message.enumRepeated[j];
633633
}

0 commit comments

Comments
 (0)