Skip to content

Commit 77fb12b

Browse files
committed
Fail if a list element is a list.
The upstream C++ parse doesn't do this, but it counts each field as a depth change/return, thus it counts lists as increasing the depth, and thus avoids stack issues via the depth limit. But since there doesn't seem to directly be a way to get a list within a list in TextFormat, just fail the parse if that happens. Also land a fuzz test generated test that caught this issue.
1 parent 793b14f commit 77fb12b

File tree

3 files changed

+42
-2
lines changed

3 files changed

+42
-2
lines changed

Sources/SwiftProtobuf/TextFormatScanner.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,7 +1289,7 @@ internal struct TextFormatScanner {
12891289
return false
12901290
}
12911291

1292-
private mutating func skipUnknownPrimativeFieldValue() throws {
1292+
private mutating func skipUnknownPrimativeFieldValue(canBeList: Bool = true) throws {
12931293
// This is modeled after the C++ text_format.cpp `SkipFieldValue()`
12941294
let c = p[0]
12951295

@@ -1301,6 +1301,10 @@ internal struct TextFormatScanner {
13011301
}
13021302

13031303
if skipOptionalBeginArray() {
1304+
guard canBeList else {
1305+
// Have encounted an array as an element in an array, that isn't legal.
1306+
throw TextFormatDecodingError.malformedText
1307+
}
13041308
if skipOptionalEndArray() {
13051309
return
13061310
}
@@ -1310,7 +1314,7 @@ internal struct TextFormatScanner {
13101314
}
13111315
let c = p[0]
13121316
if c != asciiOpenAngleBracket && c != asciiOpenCurlyBracket {
1313-
try skipUnknownPrimativeFieldValue()
1317+
try skipUnknownPrimativeFieldValue(canBeList: false)
13141318
} else {
13151319
try skipUnknownMessageFieldValue()
13161320
}

Tests/SwiftProtobufTests/Test_TextFormatDecodingOptions.swift

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,42 @@ final class Test_TextFormatDecodingOptions: XCTestCase {
690690
}
691691
}
692692

693+
func testIgnoreUnknown_FailListWithinList() {
694+
// The C++ TextFormat parse doesn't directly block this, but it calculates
695+
// recusion depth differently (it counts each field as a +1/-1 while parsing
696+
// it, that makes an array count as depth); so this got flagged by the fuzz
697+
// testing as a way would could end up with stack overflow.
698+
699+
var options = TextFormatDecodingOptions()
700+
options.ignoreUnknownFields = true
701+
options.ignoreUnknownExtensionFields = true
702+
703+
let testCases: [String] = [
704+
// fields
705+
"f:[[]]",
706+
"f:[1, [], 2]",
707+
"f <g:[[]]>",
708+
"f <g:[1, [], 2]]",
709+
// extensions
710+
"[e]:[[]]",
711+
"[e]:[1, [], 2]",
712+
"[e] <g:[[]]>",
713+
"[e] <g:[1, [], 2]]",
714+
]
715+
716+
for testCase in testCases {
717+
do {
718+
let _ = try SwiftProtoTesting_TestEmptyMessage(textFormatString: testCase,
719+
options: options)
720+
XCTFail("Should have failed - input: \(testCase)")
721+
} catch TextFormatDecodingError.malformedText {
722+
// Nothing, was the expected error
723+
} catch {
724+
XCTFail("Unexpected error: \(error) - input: \(testCase)")
725+
}
726+
}
727+
}
728+
693729
func testIgnoreUnknownWithMessageDepthLimit() {
694730
let textInput = "a: { a: { i: 1 } }"
695731

0 commit comments

Comments
 (0)