Skip to content

Commit 0d5414d

Browse files
committed
Add a fail out if raw field numbers are too long.
Follow up from the comment on adding unknown field skipping. Since there is a max field length, we can fail out if the field number is too many digits. Added tests to also cover this.
1 parent 1804a61 commit 0d5414d

File tree

3 files changed

+100
-2
lines changed

3 files changed

+100
-2
lines changed

Sources/SwiftProtobuf/TextFormatScanner.swift

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,11 @@ private let asciiLowerY = UInt8(ascii: "y")
6868
private let asciiLowerZ = UInt8(ascii: "z")
6969
private let asciiUpperZ = UInt8(ascii: "Z")
7070

71+
// https://protobuf.dev/programming-guides/proto2/#assigning
72+
// Fields can be between 1 and 536,870,911. So we can stop parsing
73+
// a raw number if we go over this (it also avoid rollover).
74+
private let maxFieldNumLength: Int = 9
75+
7176
private func fromHexDigit(_ c: UInt8) -> UInt8? {
7277
if c >= asciiZero && c <= asciiNine {
7378
return c - asciiZero
@@ -1094,9 +1099,26 @@ internal struct TextFormatScanner {
10941099
}
10951100
throw TextFormatDecodingError.unknownField
10961101
case asciiLowerA...asciiLowerZ,
1097-
asciiUpperA...asciiUpperZ,
1098-
asciiOne...asciiNine: // a...z, A...Z, 1...9
1102+
asciiUpperA...asciiUpperZ: // a...z, A...Z
10991103
return parseIdentifier()
1104+
case asciiOne...asciiNine: // 1...9 (field numbers are 123, not 0123)
1105+
let start = p
1106+
p += 1
1107+
while p != end {
1108+
let c = p[0]
1109+
if c < asciiZero || c > asciiNine {
1110+
break
1111+
}
1112+
p += 1
1113+
if p - start > maxFieldNumLength {
1114+
throw TextFormatDecodingError.malformedText
1115+
}
1116+
}
1117+
let buff = UnsafeRawBufferPointer(start: start, count: p - start)
1118+
skipWhitespace()
1119+
let s = utf8ToString(bytes: buff.baseAddress!, count: buff.count)
1120+
// Safe, can't be invalid UTF-8 given the input.
1121+
return s!
11001122
default:
11011123
throw TextFormatDecodingError.malformedText
11021124
}
@@ -1151,6 +1173,7 @@ internal struct TextFormatScanner {
11511173
// Unknown field name
11521174
break
11531175
case asciiOne...asciiNine: // 1-9 (field numbers are 123, not 0123)
1176+
let start = p
11541177
var fieldNum = Int(c) - Int(asciiZero)
11551178
p += 1
11561179
while p != end {
@@ -1161,6 +1184,9 @@ internal struct TextFormatScanner {
11611184
break
11621185
}
11631186
p += 1
1187+
if p - start > maxFieldNumLength {
1188+
throw TextFormatDecodingError.malformedText
1189+
}
11641190
}
11651191
skipWhitespace()
11661192
if names.names(for: fieldNum) != nil {

Tests/SwiftProtobufTests/Test_TextFormatDecodingOptions.swift

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,53 @@ final class Test_TextFormatDecodingOptions: XCTestCase {
638638
XCTAssertEqual(expected, msg)
639639
}
640640

641+
func testIgnoreUnknown_fieldnumTooBig() {
642+
let expected = SwiftProtoTesting_TestAllTypes.with {
643+
$0.optionalInt32 = 1
644+
$0.optionalUint32 = 2
645+
}
646+
var options = TextFormatDecodingOptions()
647+
options.ignoreUnknownFields = true
648+
649+
// The max field number is 536,870,911, so anything that takes more digits, should
650+
// fail as malformed.
651+
652+
let testCases: [(field: String, parses: Bool)] = [
653+
("536870911", true),
654+
("1536870911", false)
655+
]
656+
657+
for testCase in testCases {
658+
let text = """
659+
optional_int32: 1 # !!! real field
660+
661+
# Unknown field that's a message to test parsing of field numbers
662+
# nested within a unknown message.
663+
does_not_exist {
664+
\(testCase.field): 1
665+
}
666+
667+
optional_uint32: 2 # !!! real field
668+
"""
669+
670+
do {
671+
let msg = try SwiftProtoTesting_TestAllTypes(textFormatString: text,
672+
options: options)
673+
// If we get here, it should be the expected message.
674+
XCTAssertTrue(testCase.parses)
675+
XCTAssertEqual(msg, expected)
676+
} catch TextFormatDecodingError.malformedText {
677+
if testCase.parses {
678+
XCTFail("Unexpected malformedText - input: \(testCase.field)")
679+
} else {
680+
// Nothing, was the expected error
681+
}
682+
} catch {
683+
XCTFail("Unexpected error: \(error) - input: \(testCase.field)")
684+
}
685+
}
686+
}
687+
641688
func testIgnoreUnknownWithMessageDepthLimit() {
642689
let textInput = "a: { a: { i: 1 } }"
643690

Tests/SwiftProtobufTests/Test_TextFormat_Unknown.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,4 +386,29 @@ final class Test_TextFormat_Unknown: XCTestCase, PBTestHelpers {
386386
let textWithoutUnknowns = msg.textFormatString(options: encodeWithoutUnknowns)
387387
XCTAssertEqual(textWithoutUnknowns, "")
388388
}
389+
390+
func test_unknown_fieldnum_too_big() {
391+
// The max field number is 536,870,911, so anything that takes more digits, should
392+
// fail as malformed.
393+
394+
var opts = TextFormatDecodingOptions()
395+
opts.ignoreUnknownFields = true
396+
397+
// Max value, will pass becuase of ignoring unknowns.
398+
do {
399+
let _ = try MessageTestType(textFormatString: "536870911: 1", options: opts)
400+
} catch {
401+
XCTFail("Unexpected error: \(error)")
402+
}
403+
404+
// One more digit, should fail as malformed
405+
do {
406+
let _ = try MessageTestType(textFormatString: "1536870911: 1", options: opts)
407+
XCTFail("Shouldn't get here")
408+
} catch TextFormatDecodingError.malformedText {
409+
// This is what should have happened.
410+
} catch {
411+
XCTFail("Unexpected error: \(error)")
412+
}
413+
}
389414
}

0 commit comments

Comments
 (0)