From 930648135ead1680c93ca71b4e193003c8698099 Mon Sep 17 00:00:00 2001 From: Jonathan Flat Date: Mon, 24 Oct 2022 19:37:15 -0700 Subject: [PATCH] Check for invalid URLRequest header values --- .../FoundationNetworking/NSURLRequest.swift | 44 +++++++++++++++++++ Tests/Foundation/Tests/TestNSURLRequest.swift | 39 ++++++++++++++++ Tests/Foundation/Tests/TestURLRequest.swift | 39 ++++++++++++++++ 3 files changed, 122 insertions(+) diff --git a/Sources/FoundationNetworking/NSURLRequest.swift b/Sources/FoundationNetworking/NSURLRequest.swift index f97b7f7295..616650daec 100644 --- a/Sources/FoundationNetworking/NSURLRequest.swift +++ b/Sources/FoundationNetworking/NSURLRequest.swift @@ -472,6 +472,10 @@ open class NSMutableURLRequest : NSURLRequest { /// - Parameter value: the header field value. /// - Parameter field: the header field name (case-insensitive). open func setValue(_ value: String?, forHTTPHeaderField field: String) { + // Check that the header value is valid + if let value = value, !httpHeaderValueIsValid(value) { + return + } // Store the field name capitalized to match native Foundation let capitalizedFieldName = field.capitalized var f: [String : String] = allHTTPHeaderFields ?? [:] @@ -494,6 +498,10 @@ open class NSMutableURLRequest : NSURLRequest { /// - Parameter value: the header field value. /// - Parameter field: the header field name (case-insensitive). open func addValue(_ value: String, forHTTPHeaderField field: String) { + // Check that the header value is valid + if !httpHeaderValueIsValid(value) { + return + } // Store the field name capitalized to match native Foundation let capitalizedFieldName = field.capitalized var f: [String : String] = allHTTPHeaderFields ?? [:] @@ -571,6 +579,42 @@ open class NSMutableURLRequest : NSURLRequest { var protocolProperties: [String: Any] = [:] } +/// Checks if a header value is valid. +/// Allows header line folding, but rejects other invalid uses of CR or LF. +private func httpHeaderValueIsValid(_ value: String) -> Bool { + // Use a state machine to process the header value, transitioning on special + // characters such as CR and LF. The begin state is the accept state. CRLF + // must be followed by a SP or HTAB for valid header line folding. + enum CRLFState { + case begin + case crlf + } + var state = CRLFState.begin + for ch in value { + switch ch { + case "\0", "\r", "\n": + return false + case "\r\n": // Treated as a single Character in Swift + if state == .begin { + state = .crlf + } else { + return false + } + case " ", "\t": + if state == .crlf { + state = .begin + } + break + default: + if state != .begin { + return false + } + break + } + } + return state == .begin +} + /// Returns an existing key-value pair inside the header fields if it exists. private func existingHeaderField(_ key: String, inHeaderFields fields: [String : String]) -> (String, String)? { for (k, v) in fields { diff --git a/Tests/Foundation/Tests/TestNSURLRequest.swift b/Tests/Foundation/Tests/TestNSURLRequest.swift index a11cb71fea..1bd2641b1d 100644 --- a/Tests/Foundation/Tests/TestNSURLRequest.swift +++ b/Tests/Foundation/Tests/TestNSURLRequest.swift @@ -24,6 +24,8 @@ class TestNSURLRequest : XCTestCase { ("test_NSCoding_3", test_NSCoding_3), ("test_methodNormalization", test_methodNormalization), ("test_description", test_description), + ("test_invalidHeaderValues", test_invalidHeaderValues), + ("test_validLineFoldedHeaderValues", test_validLineFoldedHeaderValues), ] } @@ -329,4 +331,41 @@ class TestNSURLRequest : XCTestCase { XCTFail("description of nil URL should contain (null)") } } + + func test_invalidHeaderValues() { + let url = URL(string: "http://swift.org")! + let request = NSMutableURLRequest(url: url) + + let invalidHeaderValues = [ + "\r\nevil: hello\r\n\r\nGET /other HTTP/1.1\r\nevil: hello", + "invalid\0NUL", + "invalid\rCR", + "invalidCR\r", + "invalid\nLF", + "invalidLF\n", + "invalid\r\nCRLF", + "invalidCRLF\r\n", + "invalid\r\rCRCR" + ] + + for (i, value) in invalidHeaderValues.enumerated() { + request.setValue("Bar\(value)", forHTTPHeaderField: "Foo\(i)") + XCTAssertNil(request.value(forHTTPHeaderField: "Foo\(i)")) + request.addValue("Bar\(value)", forHTTPHeaderField: "Foo\(i)") + XCTAssertNil(request.value(forHTTPHeaderField: "Foo\(i)")) + } + } + + func test_validLineFoldedHeaderValues() { + let url = URL(string: "http://swift.org")! + let request = NSMutableURLRequest(url: url) + + let validHeaderValueLineFoldedTab = "Bar\r\n\tBuz" + request.setValue(validHeaderValueLineFoldedTab, forHTTPHeaderField: "FooTab") + XCTAssertEqual(request.value(forHTTPHeaderField: "FooTab"), validHeaderValueLineFoldedTab) + + let validHeaderValueLineFoldedSpace = "Bar\r\n Buz" + request.setValue(validHeaderValueLineFoldedSpace, forHTTPHeaderField: "FooSpace") + XCTAssertEqual(request.value(forHTTPHeaderField: "FooSpace"), validHeaderValueLineFoldedSpace) + } } diff --git a/Tests/Foundation/Tests/TestURLRequest.swift b/Tests/Foundation/Tests/TestURLRequest.swift index fef01c487d..af74ed9456 100644 --- a/Tests/Foundation/Tests/TestURLRequest.swift +++ b/Tests/Foundation/Tests/TestURLRequest.swift @@ -22,6 +22,8 @@ class TestURLRequest : XCTestCase { ("test_methodNormalization", test_methodNormalization), ("test_description", test_description), ("test_relativeURL", test_relativeURL), + ("test_invalidHeaderValues", test_invalidHeaderValues), + ("test_validLineFoldedHeaderValues", test_validLineFoldedHeaderValues), ] } @@ -315,4 +317,41 @@ class TestURLRequest : XCTestCase { XCTAssertEqual(nsreq.url?.absoluteURL.relativeString, "http://httpbin.org/get") XCTAssertEqual(nsreq.url?.absoluteURL.absoluteString, "http://httpbin.org/get") } + + func test_invalidHeaderValues() { + let url = URL(string: "http://swift.org")! + var request = URLRequest(url: url) + + let invalidHeaderValues = [ + "\r\nevil: hello\r\n\r\nGET /other HTTP/1.1\r\nevil: hello", + "invalid\0NUL", + "invalid\rCR", + "invalidCR\r", + "invalid\nLF", + "invalidLF\n", + "invalid\r\nCRLF", + "invalidCRLF\r\n", + "invalid\r\rCRCR" + ] + + for (i, value) in invalidHeaderValues.enumerated() { + request.setValue("Bar\(value)", forHTTPHeaderField: "Foo\(i)") + XCTAssertNil(request.value(forHTTPHeaderField: "Foo\(i)")) + request.addValue("Bar\(value)", forHTTPHeaderField: "Foo\(i)") + XCTAssertNil(request.value(forHTTPHeaderField: "Foo\(i)")) + } + } + + func test_validLineFoldedHeaderValues() { + let url = URL(string: "http://swift.org")! + var request = URLRequest(url: url) + + let validHeaderValueLineFoldedTab = "Bar\r\n\tBuz" + request.setValue(validHeaderValueLineFoldedTab, forHTTPHeaderField: "FooTab") + XCTAssertEqual(request.value(forHTTPHeaderField: "FooTab"), validHeaderValueLineFoldedTab) + + let validHeaderValueLineFoldedSpace = "Bar\r\n Buz" + request.setValue(validHeaderValueLineFoldedSpace, forHTTPHeaderField: "FooSpace") + XCTAssertEqual(request.value(forHTTPHeaderField: "FooSpace"), validHeaderValueLineFoldedSpace) + } }