swiftlang / swift-corelibs-foundation

The Foundation Project, providing core utilities, internationalization, and OS independence
swift.org
Apache License 2.0
5.3k stars 1.14k forks source link

[SR-11294] URLSession: illegal instruction when response contains "malformed" Set-Cookie headers #3985

Closed swift-ci closed 5 years ago

swift-ci commented 5 years ago
Previous ID SR-11294
Radar None
Original Reporter vzsg (JIRA User)
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Environment Swift 5.0.2 using the official swift:5.0.2 Docker image.
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 1 | |Component/s | Foundation | |Labels | Bug, Linux, RunTimeCrash | |Assignee | @millenomi | |Priority | Medium | md5: d9768b4cd9f0c26fef747e98c5ada8a6

Issue Description:

tl;dr version

When using URLSession to perform HTTP requests, there is an edge case that works on macOS, but causes the process to terminate with illegal instruction on Linux.

Set-Cookie: country=hu; path=/;

If the server response contains a trailing semicolon in a Set-Cookie header, the HTTPCookie.cookies(withResponseHeaderFields:for:) method crashes.

One liner reproduction:

HTTPCookie.cookies(withResponseHeaderFields: ["Set-Cookie": "foo=bar;"], for: URL(string: "https://foo.bar")!)

Long version

Consider the following Swift code that uses URLSession (inside):

import Foundation

do {
    let data = try Data(contentsOf: URL(string: "https://https://www.espn.com/espnradio/feeds/rss/podcast.xml?id=26832777")!)
    print(String(data: data, encoding: .utf8)!)
} catch {
    print(error)
}

On macOS, the code executes perfectly fine and prints the XML data.

On Linux (official Swift 5.0.2 docker image), the process crashes and dumps a stacktrace instead:

/usr/bin/swift[0x424a134]
/usr/bin/swift[0x4247ebe]
/usr/bin/swift[0x424a2f2]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x12890)[0x7f986e1b7890]
/usr/lib/swift/linux/libFoundation.so($s10Foundation10HTTPCookieC7cookies24withResponseHeaderFields3forSayACGSDyS2SG_AA3URLVtFZ+0x26b1)[0x7f9868e399b1]
/usr/lib/swift/linux/libFoundation.so(+0x5bb06c)[0x7f986908a06c]
/usr/lib/swift/linux/libFoundation.so(+0x5bb319)[0x7f986908a319]
/usr/lib/x86_64-linux-gnu/libcurl.so.4(+0x18b9d)[0x7f9868868b9d]
/usr/lib/x86_64-linux-gnu/libcurl.so.4(+0x16d30)[0x7f9868866d30]
/usr/lib/x86_64-linux-gnu/libcurl.so.4(+0x2a3f8)[0x7f986887a3f8]
/usr/lib/x86_64-linux-gnu/libcurl.so.4(+0x34e9b)[0x7f9868884e9b]
/usr/lib/x86_64-linux-gnu/libcurl.so.4(+0x365ca)[0x7f98688865ca]
/usr/lib/x86_64-linux-gnu/libcurl.so.4(curl_multi_socket_action+0x17)[0x7f9868886887]
/usr/lib/swift/linux/libFoundation.so(+0x5c0c5f)[0x7f986908fc5f]
/usr/lib/swift/linux/libFoundation.so(+0x5c2f89)[0x7f9869091f89]
/usr/lib/swift/linux/libFoundation.so(+0x535060)[0x7f9869004060]
/usr/lib/swift/linux/libdispatch.so(+0x341c5)[0x7f98682611c5]
/usr/lib/swift/linux/libdispatch.so(+0x29c5a)[0x7f9868256c5a]
/usr/lib/swift/linux/libdispatch.so(+0x38f71)[0x7f9868265f71]
/usr/lib/swift/linux/libdispatch.so(+0x2d7e5)[0x7f986825a7e5]
/usr/lib/swift/linux/libdispatch.so(+0x2e63e)[0x7f986825b63e]
/usr/lib/swift/linux/libdispatch.so(+0x2d7e5)[0x7f986825a7e5]
/usr/lib/swift/linux/libdispatch.so(+0x2e63e)[0x7f986825b63e]
/usr/lib/swift/linux/libdispatch.so(+0x3658a)[0x7f986826358a]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x76db)[0x7f986e1ac6db]
/lib/x86_64-linux-gnu/libc.so.6(clone+0x3f)[0x7f986c6fd88f]
Illegal instruction

The stacktrace is a bit different on 4.2.4, but still originates from the same HTTPCookie.cookies(withResponseHeaderFields:for:) method.


While inspecting the response from the mentioned URL with curl -v, the Set-Cookie headers became suspicious. Particularly, the semicolons after each line:

< Set-Cookie: country=hu; path=/;
< Set-Cookie: edition=espn-en-us; path=/; Expires=Sun, 18 Aug 2019 21:22:18 GMT;
< Set-Cookie: connectionspeed=full; path=/; Expires=Sun, 18 Aug 2019 21:22:18 GMT;
< Set-Cookie: edition-view=espn-en-us; path=/; Expires=Sun, 18 Aug 2019 21:22:18 GMT;
< Set-Cookie: SWID=2164CE80-2350-4F85-CF6F-A9D0D5D255D4; path=/; Expires=Thu, 11 Aug 2039 21:22:18 GMT; domain=espn.com;

To test this suspicion, I hacked together a small hello-world-like Node.js script (attached as index.js).

Running it on macOS while pointing the test Swift code running in Docker at host.docker.internal:3000 allowed me to test how URLSession behaves depending on the headers.

I confirmed that if the semicolon is at the end of the header, HTTPCookie crashes; and by removing the extra semicolon the crash disappears as well.

weissi commented 5 years ago

wow, this is bad. CC @millenomi/ianpartridge (JIRA User)/@spevans

spevans commented 5 years ago

Note this affects 5.0.2 but 5.1 and master are unaffected - there have been serveral PRs to fix HTTPCookie issues including parsing that went into master post 5.0.

millenomi commented 5 years ago

The 5.1 changes are standalone, IIRC. We should nominate a backport to 5.0.3. @weissi?

weissi commented 5 years ago

Awesome, the merge window is open right now. I'm more than happy to accept a backport for this.

millenomi commented 5 years ago

@weissi @spevans https://github.com/apple/swift-corelibs-foundation/pull/2467