httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Meyering <>
Subject Re: [PATCH] don't access(r/w) uri[-1] when validating resource w/empty uri string
Date Fri, 06 Jul 2012 16:11:16 GMT
Nick Kew wrote:
> On Thu, 05 Jul 2012 19:33:18 +0200
> Jim Meyering <> wrote:
> Thanks for the patch, but can you clarify?
>> At first I thought there must be code to guarantee
>> that a URI (resource->uri) has length > 0,
> In principle it must be for an HTTP request to exist.
> Have you found a testcase or code path in which it fails?

Hi Nick,

I found it via inspection.
No reproducer (if I had one, I would have posted it)

>>	 but since I found
>> similar guards against precisely that case, e.g.,
> That could be because it's necessary, but it could also be
> because the authors of those bits of code were over-cautious,
> or because the code is old and extra tests were needed when
> it was written.  The mod_dav instance is a different URI.
>> it seems best to guard the use below, too:
> Indeed, if there is a code path that leaves null or empty
> r->uri then fixing it is right.
>> From 5609908643d8456c6f56197102161e56d87e56c4 Mon Sep 17 00:00:00 2001
> Huh?  Did you send a patch in 2001?

Sure, plenty, but not that one ;-)

I cloned from git:// and posted
the output of "git format-patch".  That type of patch
always includes such a "From " line.  The Date: line
is the one that matters.

>>      uri = resource->uri;
>>      uri_len = strlen(uri);
>> -    if (uri[uri_len - 1] == '/') {
>> +    if (uri_len > 1 && uri[uri_len - 1] == '/') {
> That fixes empty uri, but segfaults on null URI.
> Makes sense if the first is possible but the second isn't.

If "uri" is NULL, then the preceding strlen would segfault,
regardless of the proposed change.  It seems pretty clear,
from all of the unprotected dereferences like that strlen,
that resource->uri cannot be NULL.  However, I could not
see anything that guarantees it is not the empty string.

View raw message