Bug 244123

Summary: libfetch: memory leak when processing multiple HTTP location response headers
Product: Base System Reporter: Hans Christian Woithe <chwoithe>
Component: binAssignee: freebsd-bugs mailing list <bugs>
Status: New ---    
Severity: Affects Some People Keywords: patch
Priority: ---    
Version: CURRENT   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
patch to libfetch
none
Simple Python server useful for testing none

Description Hans Christian Woithe 2020-02-14 14:11:18 UTC
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().
Comment 1 Hans Christian Woithe 2020-02-14 14:13:24 UTC
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.
Comment 2 Conrad Meyer freebsd_committer 2020-02-14 17:42:16 UTC
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 => ...
Comment 3 Hans Christian Woithe 2020-02-14 22:54:11 UTC
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.