Bug 244123
| Summary: | libfetch: memory leak when processing multiple HTTP location response headers | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Hans Christian Woithe <chwoithe> | ||||||
| Component: | bin | Assignee: | freebsd-bugs (Nobody) <bugs> | ||||||
| Status: | New --- | ||||||||
| Severity: | Affects Some People | Keywords: | patch | ||||||
| Priority: | --- | ||||||||
| Version: | CURRENT | ||||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
Created attachment 211647 [details]
Simple Python server useful for testing
Please find attached a simple Python server useful for testing the scenario described in the bug report.
Your fix looks correct to me. Technically this is a non-compliant server, right? IIRC, modern HTTP does not allow duplicate instances of headers. So it would also be valid for libfetch to just reject the connection when encountering the second Location. No? This case is only encountered when one HTTP response contains two location headers and not by, e.g., 302 / Location: foo => 302 / Location: bar => ... Duplicate headers can be allowed in certain situations. I tend to agree that the location header probably doesn't qualify. https://tools.ietf.org/html/rfc7230#section-3.2.2 A generic fix would involve a larger change on how headers are processed. Such a change should detect all duplicate headers that aren't allowed and return an appropriate error to the user. That could eliminate the need for the code under scrutiny. |
Created attachment 211646 [details] patch to libfetch fetchMakeURL() or fetchParseURL() are used to create a new url struct when processing a location header. In the event that the HTTP response contains multiple location headers, the previously allocated url is freed using free() instead of fetchFreeURL(). This currently prevents the struct's "doc" member from being freed. Please find attached a patch to use fetchFreeURL().