[PATCH] httpd: Pass custom HTTP headers to CGI scripts

[Alexander Vickberg]

Hi,

Thank you for your valuable feedback Assaf.

Den tors 28 mars 2019 kl 18:33 skrev Assaf Gordon:
>
> Hello Alexander,
>
> On 2019-03-28 11:04 a.m., Alexander Vickberg wrote:
> > This patch creates a list of unmatched HTTP headers and sets up
> > environment variables before running the CGI script.
>
> I assume this is inspired by my (more limited) patch
> of passing "Content-encoding" header:
> http://lists.busybox.net/pipermail/busybox/2019-March/087141.html
> (or, just a very strange timing coincidence?).
>
> I like your patch and of course, if it is accepted,
> mine isn't needed.

However unlikely it might seem I didn't see your patch until after.
Besides I wanted to be able to use completely custom headers for
which there has to be a general approach implementation.

> two small comments:
>
> > @@ -417,6 +423,7 @@ struct globals {
> > IF_FEATURE_HTTPD_CGI(char *host;)
> > IF_FEATURE_HTTPD_CGI(char *http_accept;)
> > IF_FEATURE_HTTPD_CGI(char *http_accept_language;)
> > +IF_FEATURE_HTTPD_CGI(HTTP_Header *hdr_list;)
>
> Since your mechanism is now much more generic than
> the hard-coded CGI headers, perhaps they can
> be safely removed?
> i.e. host/http_accept/http_accept_language/cookie/referer .
>
> Seems like this could save some space.

Good point. I agree.

> > +HTTP_Header *cur = xzalloc(sizeof(HTTP_Header));
> > +char *after_colon = strchr(iobuf, ':');
> > +char *ch = iobuf;
> > +
> > +if (!after_colon)
> > +    continue;
> > +
>
> I think the combination of "xzalloc" + "continue"
> opens the possibility of a resource leak -
> if a malicious client sends lots of HTTP header lines without
> a colon, there's no corresponding "free".

Another good catch. I also found another issue that if the requested URL is
not a CGI then the list will be populated but never freed. However I see
other
places which do allocs or strdups and not freeing them. I wonder if that
behavior is counting on that the fork handling the request will exit and
that
then the kernel will free it for us?

> regards,
>   - assaf

/Alexander
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20190329/47f67041/attachment-0001.html>