Re: wget2 | Draft: Small fixes (!505)

[@rockdaboot]

Tim Rühsen started a new discussion on libwget/buffer.c: 
https://gitlab.com/gnuwget/wget2/-/merge_requests/505#note_963563313

>               char *start = buf->data;
>               char *end = start + buf->length - 1;
>  
> -             if (isspace(*end)) {
> +             /* Accessing `start - 1` is undefined so leave if `start == 
> end` */

Awesome, great finding !

My analysis is slightly different - there is a buffer overflow (in fact it is a 
"underflow" =D) due to a wrong order of the checks in the for loop. We should 
first check whether `end >= start` before we access `*end`.

So my suggestion is this patch
```
index 781aec3f9..b69eefa75 100644
--- a/libwget/buffer.c
+++ b/libwget/buffer.c
@@ -550,7 +550,7 @@ char *wget_buffer_trim(wget_buffer *buf)
 
                if (isspace(*end)) {
                        /* Skip trailing spaces */
-                       for (; isspace(*end) && end >= start; end--)
+                       for (; end >= start && isspace(*end); end--)
                                ;
                        end[1] = 0;
                        buf->length = (size_t) (end - start + 1);
@@ -558,7 +558,7 @@ char *wget_buffer_trim(wget_buffer *buf)
 
                if (isspace(*start)) {
                        /* Skip leading spaces */
-                       for (; isspace(*start) && end >= start; start++)
+                       for (; end >= start && isspace(*start); start++)
                                ;
                        buf->length = (size_t) (end - start + 1);
                        /* Include trailing 0 */
```

-- 
Reply to this email directly or view it on GitLab: 
https://gitlab.com/gnuwget/wget2/-/merge_requests/505#note_963563313
You're receiving this email because of your account on gitlab.com.