From 218ed4179bc348bbdee4985f0a3678f6e1ac60b2 Mon Sep 17 00:00:00 2001 From: 8de2fdb0 <89734465+8de2fdb0@users.noreply.github.com> Date: Wed, 4 Oct 2023 11:11:21 +0000 Subject: [PATCH 1/3] feat: add bigint input validation Input validation for too large int64 and uint64 values when using the compiler option forceLong=bigint. --- integration/map-bigint-optional/test.ts | 6 +++ .../google/protobuf/timestamp.ts | 3 ++ .../google/protobuf/wrappers.ts | 6 +++ .../numbers-long-string-test.ts | 53 +++++++++++++++++++ integration/simple-long-bigint/simple.ts | 18 +++++++ src/main.ts | 50 ++++++++++++++++- 6 files changed, 134 insertions(+), 2 deletions(-) diff --git a/integration/map-bigint-optional/test.ts b/integration/map-bigint-optional/test.ts index 83b3163f2..5a7977640 100644 --- a/integration/map-bigint-optional/test.ts +++ b/integration/map-bigint-optional/test.ts @@ -130,9 +130,15 @@ function createBaseMapBigInt_MapEntry(): MapBigInt_MapEntry { export const MapBigInt_MapEntry = { encode(message: MapBigInt_MapEntry, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer { if (message.key !== BigInt("0")) { + if (BigInt.asUintN(64, message.key) !== message.key) { + throw new Error("value provided for field message.key of type fixed64 too large"); + } writer.uint32(9).fixed64(message.key.toString()); } if (message.value !== BigInt("0")) { + if (BigInt.asIntN(64, message.value) !== message.value) { + throw new Error("value provided for field message.value of type int64 too large"); + } writer.uint32(16).int64(message.value.toString()); } if (message._unknownFields !== undefined) { diff --git a/integration/simple-long-bigint/google/protobuf/timestamp.ts b/integration/simple-long-bigint/google/protobuf/timestamp.ts index 7808d3a03..f005cadf4 100644 --- a/integration/simple-long-bigint/google/protobuf/timestamp.ts +++ b/integration/simple-long-bigint/google/protobuf/timestamp.ts @@ -118,6 +118,9 @@ function createBaseTimestamp(): Timestamp { export const Timestamp = { encode(message: Timestamp, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer { if (message.seconds !== BigInt("0")) { + if (BigInt.asIntN(64, message.seconds) !== message.seconds) { + throw new Error("value provided for field message.seconds of type int64 too large"); + } writer.uint32(8).int64(message.seconds.toString()); } if (message.nanos !== 0) { diff --git a/integration/simple-long-bigint/google/protobuf/wrappers.ts b/integration/simple-long-bigint/google/protobuf/wrappers.ts index 2f9fc2ab9..c1d6b8f8a 100644 --- a/integration/simple-long-bigint/google/protobuf/wrappers.ts +++ b/integration/simple-long-bigint/google/protobuf/wrappers.ts @@ -215,6 +215,9 @@ function createBaseInt64Value(): Int64Value { export const Int64Value = { encode(message: Int64Value, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer { if (message.value !== BigInt("0")) { + if (BigInt.asIntN(64, message.value) !== message.value) { + throw new Error("value provided for field message.value of type int64 too large"); + } writer.uint32(8).int64(message.value.toString()); } return writer; @@ -272,6 +275,9 @@ function createBaseUInt64Value(): UInt64Value { export const UInt64Value = { encode(message: UInt64Value, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer { if (message.value !== BigInt("0")) { + if (BigInt.asUintN(64, message.value) !== message.value) { + throw new Error("value provided for field message.value of type uint64 too large"); + } writer.uint32(8).uint64(message.value.toString()); } return writer; diff --git a/integration/simple-long-bigint/numbers-long-string-test.ts b/integration/simple-long-bigint/numbers-long-string-test.ts index 35bff0889..1f6e6203f 100644 --- a/integration/simple-long-bigint/numbers-long-string-test.ts +++ b/integration/simple-long-bigint/numbers-long-string-test.ts @@ -7,6 +7,12 @@ import PbNumbers = pbjs.Numbers; import UInt64Value = google.protobuf.UInt64Value; import PbTimestamp = google.protobuf.Timestamp; + +// 18_446_744_073_709_551_615n +const MAX_UINT64 = BigInt("18446744073709551615") +// 9_223_372_036_854_775_807n +const MAX_INT64 = BigInt("9223372036854775807") + describe("number", () => { it("generates types correctly", () => { const simple: Numbers = { @@ -168,4 +174,51 @@ describe("number", () => { } `); }); + + it('throws error on too large bigints', () => { + const testCases = [ + { + failValue: { int64: MAX_INT64 + BigInt("1") }, + failMsg: "value provided for field message.int64 of type int64 too large", + passValue: { int64: MAX_INT64 }, + }, + { + failValue: { uint64: MAX_UINT64 + BigInt("1") }, + failMsg: "value provided for field message.uint64 of type uint64 too large", + passValue: { uint64: MAX_UINT64 }, + }, + { + failValue: { sint64: MAX_INT64 + BigInt("1") }, + failMsg: "value provided for field message.sint64 of type sint64 too large", + passValue: { sint64: MAX_INT64 }, + }, + { + failValue: { fixed64: MAX_UINT64 + BigInt("1") }, + failMsg: "value provided for field message.fixed64 of type fixed64 too large", + passValue: { fixed64: MAX_UINT64 }, + }, + { + failValue: { sfixed64: MAX_INT64 + BigInt("1") }, + failMsg: "value provided for field message.sfixed64 of type sfixed64 too large", + passValue: { sfixed64: MAX_INT64 }, + }, + { + failValue: { guint64: MAX_UINT64 + BigInt("1") }, + failMsg: "value provided for field message.value of type uint64 too large", + passValue: { guint64: MAX_UINT64 }, + }, + { + failValue: { uint64s: [MAX_UINT64 + BigInt("1")] }, + failMsg: "a value provided in array field uint64s of type uint64 is too large", + passValue: { uint64s: [MAX_UINT64] }, + } + ]; + + for (const testCase of testCases) { + const failValue = Numbers.fromPartial(testCase.failValue); + expect(() => { Numbers.encode(failValue) }).toThrow(testCase.failMsg); + const passValue = Numbers.fromPartial(testCase.passValue); + expect(() => Numbers.encode(passValue).finish()).not.toThrowError(); + } + }); }); diff --git a/integration/simple-long-bigint/simple.ts b/integration/simple-long-bigint/simple.ts index 9605f5862..c65d8cbf7 100644 --- a/integration/simple-long-bigint/simple.ts +++ b/integration/simple-long-bigint/simple.ts @@ -56,30 +56,45 @@ export const Numbers = { writer.uint32(24).int32(message.int32); } if (message.int64 !== BigInt("0")) { + if (BigInt.asIntN(64, message.int64) !== message.int64) { + throw new Error("value provided for field message.int64 of type int64 too large"); + } writer.uint32(32).int64(message.int64.toString()); } if (message.uint32 !== 0) { writer.uint32(40).uint32(message.uint32); } if (message.uint64 !== BigInt("0")) { + if (BigInt.asUintN(64, message.uint64) !== message.uint64) { + throw new Error("value provided for field message.uint64 of type uint64 too large"); + } writer.uint32(48).uint64(message.uint64.toString()); } if (message.sint32 !== 0) { writer.uint32(56).sint32(message.sint32); } if (message.sint64 !== BigInt("0")) { + if (BigInt.asIntN(64, message.sint64) !== message.sint64) { + throw new Error("value provided for field message.sint64 of type sint64 too large"); + } writer.uint32(64).sint64(message.sint64.toString()); } if (message.fixed32 !== 0) { writer.uint32(77).fixed32(message.fixed32); } if (message.fixed64 !== BigInt("0")) { + if (BigInt.asUintN(64, message.fixed64) !== message.fixed64) { + throw new Error("value provided for field message.fixed64 of type fixed64 too large"); + } writer.uint32(81).fixed64(message.fixed64.toString()); } if (message.sfixed32 !== 0) { writer.uint32(93).sfixed32(message.sfixed32); } if (message.sfixed64 !== BigInt("0")) { + if (BigInt.asIntN(64, message.sfixed64) !== message.sfixed64) { + throw new Error("value provided for field message.sfixed64 of type sfixed64 too large"); + } writer.uint32(97).sfixed64(message.sfixed64.toString()); } if (message.guint64 !== undefined) { @@ -90,6 +105,9 @@ export const Numbers = { } writer.uint32(122).fork(); for (const v of message.uint64s) { + if (BigInt.asUintN(64, v) !== v) { + throw new Error("a value provided in array field uint64s of type uint64 is too large"); + } writer.uint64(v.toString()); } writer.ldelim(); diff --git a/src/main.ts b/src/main.ts index 9033b2806..0627979ed 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1240,7 +1240,21 @@ function getEncodeWriteSnippet(ctx: Context, field: FieldDescriptorProto): (plac return (place) => code`writer.uint32(${tag}).${toReaderCall(field)}(${toNumber}(${place}))`; } else if (isLong(field) && options.forceLong === LongOption.BIGINT) { const tag = ((field.number << 3) | basicWireType(field.type)) >>> 0; - return (place) => code`writer.uint32(${tag}).${toReaderCall(field)}(${place}.toString())`; + const fieldType = toReaderCall(field); + switch (fieldType) { + case 'int64': case 'sint64': case 'sfixed64': + return (place) => code`if (BigInt.asIntN(64, ${place}) !== ${place}) { + throw new Error('value provided for field ${place} of type ${fieldType} too large'); + } + writer.uint32(${tag}).${toReaderCall(field)}(${place}.toString())`; + case 'uint64': case 'fixed64': + return (place) => code`if (BigInt.asUintN(64, ${place}) !== ${place}) { + throw new Error('value provided for field ${place} of type ${fieldType} too large'); + } + writer.uint32(${tag}).${toReaderCall(field)}(${place}.toString())`; + default: + throw new Error(`unexpected BigInt type: ${fieldType}`); + } } else if (isScalar(field) || isEnum(field)) { const tag = ((field.number << 3) | basicWireType(field.type)) >>> 0; return (place) => code`writer.uint32(${tag}).${toReaderCall(field)}(${place})`; @@ -1381,13 +1395,45 @@ function generateEncode(ctx: Context, fullName: string, messageDesc: DescriptorP // Ideally we'd reuse `writeSnippet` but it has tagging embedded inside of it. const tag = ((field.number << 3) | 2) >>> 0; const rhs = (x: string) => (isLong(field) && options.forceLong === LongOption.BIGINT ? `${x}.toString()` : x); - const listWriteSnippet = code` + let listWriteSnippet = code` writer.uint32(${tag}).fork(); for (const v of message.${fieldName}) { writer.${toReaderCall(field)}(${rhs("v")}); } writer.ldelim(); `; + + if (options.forceLong === LongOption.BIGINT) { + const fieldType = toReaderCall(field); + switch (fieldType) { + case 'int64': case 'sint64': case 'sfixed64': + listWriteSnippet = code` + writer.uint32(${tag}).fork(); + for (const v of message.${fieldName}) { + if (BigInt.asIntN(64, v) !== v) { + throw new Error('a value provided in array field ${fieldName} of type ${fieldType} is too large'); + } + writer.${toReaderCall(field)}(${rhs("v")}); + } + writer.ldelim(); + `; + break; + case 'uint64': case 'fixed64': + listWriteSnippet = code` + writer.uint32(${tag}).fork(); + for (const v of message.${fieldName}) { + if (BigInt.asUintN(64, v) !== v) { + throw new Error('a value provided in array field ${fieldName} of type ${fieldType} is too large'); + } + writer.${toReaderCall(field)}(${rhs("v")}); + } + writer.ldelim(); + `; + break; + default: + throw new Error(`unexpected BigInt type: ${fieldType}`); + } + } if (isOptional) { chunks.push(code` if (message.${fieldName} !== undefined && message.${fieldName}.length !== 0) { From 2fc4459cbede9b271f28d4aa9cfbc743af55b5fb Mon Sep 17 00:00:00 2001 From: 8de2fdb0 <89734465+8de2fdb0@users.noreply.github.com> Date: Wed, 4 Oct 2023 16:22:35 +0200 Subject: [PATCH 2/3] fix: test if field isLong for array bigint input validation --- src/main.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.ts b/src/main.ts index 0627979ed..7ab33601f 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1403,7 +1403,7 @@ function generateEncode(ctx: Context, fullName: string, messageDesc: DescriptorP writer.ldelim(); `; - if (options.forceLong === LongOption.BIGINT) { + if (isLong(field) && options.forceLong === LongOption.BIGINT) { const fieldType = toReaderCall(field); switch (fieldType) { case 'int64': case 'sint64': case 'sfixed64': From 520e1e4baf1f3f3746f6ced26acb7fda9dcf9624 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Wed, 4 Oct 2023 19:43:08 -0500 Subject: [PATCH 3/3] Format. --- src/main.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/main.ts b/src/main.ts index 7ab33601f..9f67dc73a 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1242,12 +1242,15 @@ function getEncodeWriteSnippet(ctx: Context, field: FieldDescriptorProto): (plac const tag = ((field.number << 3) | basicWireType(field.type)) >>> 0; const fieldType = toReaderCall(field); switch (fieldType) { - case 'int64': case 'sint64': case 'sfixed64': + case "int64": + case "sint64": + case "sfixed64": return (place) => code`if (BigInt.asIntN(64, ${place}) !== ${place}) { throw new Error('value provided for field ${place} of type ${fieldType} too large'); } writer.uint32(${tag}).${toReaderCall(field)}(${place}.toString())`; - case 'uint64': case 'fixed64': + case "uint64": + case "fixed64": return (place) => code`if (BigInt.asUintN(64, ${place}) !== ${place}) { throw new Error('value provided for field ${place} of type ${fieldType} too large'); } @@ -1406,7 +1409,9 @@ function generateEncode(ctx: Context, fullName: string, messageDesc: DescriptorP if (isLong(field) && options.forceLong === LongOption.BIGINT) { const fieldType = toReaderCall(field); switch (fieldType) { - case 'int64': case 'sint64': case 'sfixed64': + case "int64": + case "sint64": + case "sfixed64": listWriteSnippet = code` writer.uint32(${tag}).fork(); for (const v of message.${fieldName}) { @@ -1418,7 +1423,8 @@ function generateEncode(ctx: Context, fullName: string, messageDesc: DescriptorP writer.ldelim(); `; break; - case 'uint64': case 'fixed64': + case "uint64": + case "fixed64": listWriteSnippet = code` writer.uint32(${tag}).fork(); for (const v of message.${fieldName}) {