Skip to content

Commit 34fd1d2

Browse files
authored
Check for invalid URLRequest header values (#4650) (#4686)
Co-authored-by: Jonathan Flat <[email protected]> Co-authored-by: Jonathan Flat <[email protected]>
1 parent 7f08595 commit 34fd1d2

File tree

3 files changed

+122
-0
lines changed

3 files changed

+122
-0
lines changed

Sources/FoundationNetworking/NSURLRequest.swift

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,10 @@ open class NSMutableURLRequest : NSURLRequest {
472472
/// - Parameter value: the header field value.
473473
/// - Parameter field: the header field name (case-insensitive).
474474
open func setValue(_ value: String?, forHTTPHeaderField field: String) {
475+
// Check that the header value is valid
476+
if let value = value, !httpHeaderValueIsValid(value) {
477+
return
478+
}
475479
// Store the field name capitalized to match native Foundation
476480
let capitalizedFieldName = field.capitalized
477481
var f: [String : String] = allHTTPHeaderFields ?? [:]
@@ -494,6 +498,10 @@ open class NSMutableURLRequest : NSURLRequest {
494498
/// - Parameter value: the header field value.
495499
/// - Parameter field: the header field name (case-insensitive).
496500
open func addValue(_ value: String, forHTTPHeaderField field: String) {
501+
// Check that the header value is valid
502+
if !httpHeaderValueIsValid(value) {
503+
return
504+
}
497505
// Store the field name capitalized to match native Foundation
498506
let capitalizedFieldName = field.capitalized
499507
var f: [String : String] = allHTTPHeaderFields ?? [:]
@@ -571,6 +579,42 @@ open class NSMutableURLRequest : NSURLRequest {
571579
var protocolProperties: [String: Any] = [:]
572580
}
573581

582+
/// Checks if a header value is valid.
583+
/// Allows header line folding, but rejects other invalid uses of CR or LF.
584+
private func httpHeaderValueIsValid(_ value: String) -> Bool {
585+
// Use a state machine to process the header value, transitioning on special
586+
// characters such as CR and LF. The begin state is the accept state. CRLF
587+
// must be followed by a SP or HTAB for valid header line folding.
588+
enum CRLFState {
589+
case begin
590+
case crlf
591+
}
592+
var state = CRLFState.begin
593+
for ch in value {
594+
switch ch {
595+
case "\0", "\r", "\n":
596+
return false
597+
case "\r\n": // Treated as a single Character in Swift
598+
if state == .begin {
599+
state = .crlf
600+
} else {
601+
return false
602+
}
603+
case " ", "\t":
604+
if state == .crlf {
605+
state = .begin
606+
}
607+
break
608+
default:
609+
if state != .begin {
610+
return false
611+
}
612+
break
613+
}
614+
}
615+
return state == .begin
616+
}
617+
574618
/// Returns an existing key-value pair inside the header fields if it exists.
575619
private func existingHeaderField(_ key: String, inHeaderFields fields: [String : String]) -> (String, String)? {
576620
for (k, v) in fields {

Tests/Foundation/Tests/TestNSURLRequest.swift

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ class TestNSURLRequest : XCTestCase {
2424
("test_NSCoding_3", test_NSCoding_3),
2525
("test_methodNormalization", test_methodNormalization),
2626
("test_description", test_description),
27+
("test_invalidHeaderValues", test_invalidHeaderValues),
28+
("test_validLineFoldedHeaderValues", test_validLineFoldedHeaderValues),
2729
]
2830
}
2931

@@ -329,4 +331,41 @@ class TestNSURLRequest : XCTestCase {
329331
XCTFail("description of nil URL should contain (null)")
330332
}
331333
}
334+
335+
func test_invalidHeaderValues() {
336+
let url = URL(string: "http://swift.org")!
337+
let request = NSMutableURLRequest(url: url)
338+
339+
let invalidHeaderValues = [
340+
"\r\nevil: hello\r\n\r\nGET /other HTTP/1.1\r\nevil: hello",
341+
"invalid\0NUL",
342+
"invalid\rCR",
343+
"invalidCR\r",
344+
"invalid\nLF",
345+
"invalidLF\n",
346+
"invalid\r\nCRLF",
347+
"invalidCRLF\r\n",
348+
"invalid\r\rCRCR"
349+
]
350+
351+
for (i, value) in invalidHeaderValues.enumerated() {
352+
request.setValue("Bar\(value)", forHTTPHeaderField: "Foo\(i)")
353+
XCTAssertNil(request.value(forHTTPHeaderField: "Foo\(i)"))
354+
request.addValue("Bar\(value)", forHTTPHeaderField: "Foo\(i)")
355+
XCTAssertNil(request.value(forHTTPHeaderField: "Foo\(i)"))
356+
}
357+
}
358+
359+
func test_validLineFoldedHeaderValues() {
360+
let url = URL(string: "http://swift.org")!
361+
let request = NSMutableURLRequest(url: url)
362+
363+
let validHeaderValueLineFoldedTab = "Bar\r\n\tBuz"
364+
request.setValue(validHeaderValueLineFoldedTab, forHTTPHeaderField: "FooTab")
365+
XCTAssertEqual(request.value(forHTTPHeaderField: "FooTab"), validHeaderValueLineFoldedTab)
366+
367+
let validHeaderValueLineFoldedSpace = "Bar\r\n Buz"
368+
request.setValue(validHeaderValueLineFoldedSpace, forHTTPHeaderField: "FooSpace")
369+
XCTAssertEqual(request.value(forHTTPHeaderField: "FooSpace"), validHeaderValueLineFoldedSpace)
370+
}
332371
}

Tests/Foundation/Tests/TestURLRequest.swift

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ class TestURLRequest : XCTestCase {
2222
("test_methodNormalization", test_methodNormalization),
2323
("test_description", test_description),
2424
("test_relativeURL", test_relativeURL),
25+
("test_invalidHeaderValues", test_invalidHeaderValues),
26+
("test_validLineFoldedHeaderValues", test_validLineFoldedHeaderValues),
2527
]
2628
}
2729

@@ -315,4 +317,41 @@ class TestURLRequest : XCTestCase {
315317
XCTAssertEqual(nsreq.url?.absoluteURL.relativeString, "http://httpbin.org/get")
316318
XCTAssertEqual(nsreq.url?.absoluteURL.absoluteString, "http://httpbin.org/get")
317319
}
320+
321+
func test_invalidHeaderValues() {
322+
let url = URL(string: "http://swift.org")!
323+
var request = URLRequest(url: url)
324+
325+
let invalidHeaderValues = [
326+
"\r\nevil: hello\r\n\r\nGET /other HTTP/1.1\r\nevil: hello",
327+
"invalid\0NUL",
328+
"invalid\rCR",
329+
"invalidCR\r",
330+
"invalid\nLF",
331+
"invalidLF\n",
332+
"invalid\r\nCRLF",
333+
"invalidCRLF\r\n",
334+
"invalid\r\rCRCR"
335+
]
336+
337+
for (i, value) in invalidHeaderValues.enumerated() {
338+
request.setValue("Bar\(value)", forHTTPHeaderField: "Foo\(i)")
339+
XCTAssertNil(request.value(forHTTPHeaderField: "Foo\(i)"))
340+
request.addValue("Bar\(value)", forHTTPHeaderField: "Foo\(i)")
341+
XCTAssertNil(request.value(forHTTPHeaderField: "Foo\(i)"))
342+
}
343+
}
344+
345+
func test_validLineFoldedHeaderValues() {
346+
let url = URL(string: "http://swift.org")!
347+
var request = URLRequest(url: url)
348+
349+
let validHeaderValueLineFoldedTab = "Bar\r\n\tBuz"
350+
request.setValue(validHeaderValueLineFoldedTab, forHTTPHeaderField: "FooTab")
351+
XCTAssertEqual(request.value(forHTTPHeaderField: "FooTab"), validHeaderValueLineFoldedTab)
352+
353+
let validHeaderValueLineFoldedSpace = "Bar\r\n Buz"
354+
request.setValue(validHeaderValueLineFoldedSpace, forHTTPHeaderField: "FooSpace")
355+
XCTAssertEqual(request.value(forHTTPHeaderField: "FooSpace"), validHeaderValueLineFoldedSpace)
356+
}
318357
}

0 commit comments

Comments
 (0)