Skip to content

New XML PropertyListDecoder parser accepts malformed tags #1180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 35 additions & 23 deletions Sources/FoundationEssentials/PropertyList/XMLPlistScanner.swift
Original file line number Diff line number Diff line change
Expand Up @@ -908,68 +908,80 @@ internal struct XMLPlistScanner {
}
}

func determineTag() throws -> XMLPlistTag? {
mutating func readTag() throws -> XMLPlistTag? {
let marker = reader.readIndex
switch reader.char(at: marker) {
var tag: XMLPlistTag?
switch reader.peek() {
case UInt8(ascii: "a"): // Array
if matches(tag: .array, at: marker, until: reader.endIndex) {
return .array
tag = .array
}
case UInt8(ascii: "d"): // Dictionary, data, or date
if matches(tag: .dict, at: marker, until: reader.endIndex) {
return .dict
tag = .dict
} else if matches(tag: .data, at: marker, until: reader.endIndex) {
return .data
tag = .data
} else if matches(tag: .date, at: marker, until: reader.endIndex) {
return .date
tag = .date
}
case UInt8(ascii: "f"): // false (boolean)
if matches(tag: .false, at: marker, until: reader.endIndex) {
return .false
tag = .false
}
case UInt8(ascii: "i"): // integer
if matches(tag: .integer, at: marker, until: reader.endIndex) {
return .integer
tag = .integer
}
case UInt8(ascii: "k"): // Key of a dictionary
if matches(tag: .key, at: marker, until: reader.endIndex) {
return .key
tag = .key
}
case UInt8(ascii: "p"): // Plist
if matches(tag: .plist, at: marker, until: reader.endIndex) {
return .plist
tag = .plist
}
case UInt8(ascii: "r"): // real
if matches(tag: .real, at: marker, until: reader.endIndex) {
return .real
tag = .real
}
case UInt8(ascii: "s"): // String
if matches(tag: .string, at: marker, until: reader.endIndex) {
return .string
tag = .string
}
case UInt8(ascii: "t"): // true (boolean)
if matches(tag: .true, at: marker, until: reader.endIndex) {
return .true
tag = .true
}
case ._space, ._tab, ._newline, ._return, ._closeangle:
throw XMLPlistError.malformedTag(line: reader.lineNumber)
default:
break
}
return nil

guard let tag else {
return nil
}

// Tag names must be delimited either by whitespace or a '>' or `/` character to be valid.
reader.advance(tag.tagLength)
switch reader.peek() {
case ._closeangle, ._forwardslash, ._space, ._tab, ._newline, ._return:
return tag
default:
return nil
}
}

mutating func peekXMLElement() throws -> (XMLPlistTag, isEmpty: Bool) {
guard let tag = try determineTag() else {
let badTagStart = reader.readIndex
while let ch = reader.read(), ch != ._closeangle { }
let badTagEnd = reader.readIndex
let markerStr = String._tryFromUTF8(reader.fullBuffer[badTagStart..<badTagEnd]) ?? "<unparseable>"
let tagStart = reader.readIndex
guard let tag = try readTag() else {
var idx = reader.readIndex
while idx < reader.endIndex, reader.char(at: idx) != ._closeangle {
reader.advance(&idx)
}
let markerStr = String._tryFromUTF8(reader.fullBuffer[tagStart..<idx]) ?? "<unparseable>"
throw XMLPlistError.other("Encountered unknown tag \(markerStr) on line \(reader.lineNumber)")
}

reader.advance(tag.tagLength)

// Skip past any characters (whitespace or attributes) that may follow a valid tag name.
while let ch = reader.read(), ch != ._closeangle { }
if reader.isAtEnd {
throw XMLPlistError.malformedTag(line: reader.lineNumber)
Expand Down
11 changes: 11 additions & 0 deletions Tests/FoundationEssentialsTests/PropertyListEncoderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1545,6 +1545,17 @@ data1 = <7465
}

}

func test_garbageCharactersAfterXMLTagName() throws {
let garbage = "<plist><dict><key>bar</key><stringGARBAGE>foo</string></dict></plist>".data(using: .utf8)!

XCTAssertThrowsError(try PropertyListDecoder().decode([String:String].self, from: garbage))

// Historical behavior allows for whitespace to immediately follow tag names
let acceptable = "<plist><dict><key>bar</key><string >foo</string></dict></plist>".data(using: .utf8)!

XCTAssertEqual(try PropertyListDecoder().decode([String:String].self, from: acceptable), ["bar":"foo"])
}
}


Expand Down