On 4/15/22 4:21 PM, Stefan Eissing wrote:
>
>
>> Am 15.04.2022 um 15:24 schrieb Yann Ylavic <ylavic....@gmail.com>:
>>
>> On Wed, Apr 6, 2022 at 11:17 AM <ic...@apache.org> wrote:
>>>
>>> Modified: httpd/httpd/trunk/server/util.c
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1899609&r1=1899608&r2=1899609&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/server/util.c (original)
>>> +++ httpd/httpd/trunk/server/util.c Wed Apr 6 09:17:42 2022
>>> @@ -2615,7 +2615,7 @@ AP_DECLARE(void) ap_content_type_tolower
>>> */
>>> AP_DECLARE(char *) ap_escape_quotes(apr_pool_t *p, const char *instring)
>>> {
>>> - int newlen = 0;
>>> + apr_ssize_t extra = 0;
>>
>> Shouldn't it be an apr_size_t?
>
> Similar comment raised on the PR https://github.com/apache/httpd/pull/298
>
> Not totally sure. The thing is that C in general has a problem with
> strings where ptrdiff_t (apr_ssize_t) is not sufficient. Allocating something
> larger than ptridff_t leads to undefined behaviour.
>
> So, maybe we should check that "(inchr - instring) + extra + 1" does not
> wrap around?
I did some tests with gcc 8.5 and it seems to do the "expected" thing if two
pointers are farther away than PTRDIFF_MAX:
It provides the correct difference. This does not work if the difference would
be negative (just swapped the pointers for the test).
Having said this the above does not mean that other compilers or other versions
of gcc behave the same as the specs for C do not
define a behavior in this case. I guess the best thing we can do is:
1. Make extra apr_size_t
2. Check that inchr - instring remains >= 0 .
3. Store inchr - instring in an apr_size_t.
4. Check that inchr - instring <= SIZE_MAX - extra - 1
5. If 2. and 4. are true calculate (inchr - instring) + extra + 1 otherwise
abort() as at least 4 means we would be OOM anyway and
2. does not offer a sane way to us to solve this.
Regards
Rüdiger
