On Mon, 16 Jul 2018 13:35:14 -0700
Aaron Burrow <burr...@charstarstar.com> wrote:
Hey Aaron,
> quark has a 1 byte NULL stack overflow triggered by an HTTP request
> for a folder path not ending with '/' and having length PATH_MAX-1.
> Relevant code from http.c:http_send_response()
>
> if (S_ISDIR(st.st_mode)) {
> /* add / to target if not present */
> len = strlen(realtarget);
> if (len == PATH_MAX - 2) {
> return http_send_status(fd, S_REQUEST_TOO_LARGE);
> }
> if (len && realtarget[len - 1] != '/') {
> realtarget[len] = '/';
> realtarget[len + 1] = '\0';
> }
> }
>
> The issue is mitigated by changing the second conditional to
>
> if (len >= PATH_MAX - 2) {
>
> There are two ways to trigger the bug. On my system, PATH_MAX = 4096
> and the default quark config has HEADER_MAX = 4096. The bug is
> triggered by folder paths of length 4095. The `-m` option lets me
> increase the path length despite the HEADER_MAX constraint.
> [...]
> Alternatively, the bug can be triggered without `-m` if the system
> has a 'small' PATH_MAX or the configuration has a 'large' HEADER_MAX.
> [...]
> patch
> [...]
I am genuinely impressed by the detail you gave in this mail! Very
nice write-up and report with an included fix. Very well done!
Yes, quark still needs an audit before its first release and there are
probably more bugs like this floating around, however, if more bugs
are present they'll mostly be within these "few" URL-modification and
handling sections. The HTTP parser itself is probably safe and I'd
trust it with my life to be honest.
Thank you very much for the report and sending in a fix. It's really
appreciated. I have applied[0] your patch and attributed you accordingly
in the LICENSE.
If you find any more problems, please let me know! Thanks for using and
testing quark.
With best regards
Laslo Hunhold
[0]:https://git.suckless.org/quark/commit/?id=d2013a6337972c62a71f01324e87af0e55579245
--
Laslo Hunhold <d...@frign.de>
