[PATCH 0/1] MR7356: http.sys: Avoid crashes in complete_irp() if http header value is space

[Wei Xie (＠wayne)]

The value_len returned by the parse_header function may be less than 0

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7356

[PATCH 1/1] http.sys: Avoid crashes in complete_irp() if http header value is space

[Wei Xie]

From: Wei Xie <xiewei(a)uniontech.com>

The value_len returned by the parse_header function may be less than 0
---
 dlls/http.sys/http.c    | 2 +-
 dlls/http.sys/request.h | 9 ++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/dlls/http.sys/http.c b/dlls/http.sys/http.c
index edf099b43ab..d4a85522aaa 100644
--- a/dlls/http.sys/http.c
+++ b/dlls/http.sys/http.c
@@ -267,7 +267,7 @@ static void parse_header(const char *name, int *name_len, const char **value, in
     while (*p == ' ' || *p == '\t') ++p;
     *value = p;
     while (isprint(*p) || *p == '\t') ++p;
-    while (isspace(*p)) --p; /* strip trailing LWS */
+    while (p >= *value && isspace(*p)) --p; /* strip trailing LWS */
     *value_len = p - *value + 1;
 }
 
diff --git a/dlls/http.sys/request.h b/dlls/http.sys/request.h
index f307ca3dead..4ca3202b8f6 100644
--- a/dlls/http.sys/request.h
+++ b/dlls/http.sys/request.h
@@ -259,12 +259,15 @@ static NTSTATUS complete_irp(struct connection *conn, IRP *irp)
             offset += name_len;
             buffer[offset++] = 0;
             unk_headers[unk_header_idx].pRawValue = params.addr + offset;
-            memcpy(buffer + offset, value, value_len);
-            offset += value_len;
+            if (value_len > 0)
+            {
+                memcpy(buffer + offset, value, value_len);
+                offset += value_len;
+            }
             buffer[offset++] = 0;
             ++unk_header_idx;
         }
-        else
+        else if (value_len > 0)
         {
             req->Headers.KnownHeaders[id].RawValueLength = value_len;
             req->Headers.KnownHeaders[id].pRawValue = params.addr + offset;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/7356

[Elizabeth Figura (＠zfigura)]

This seems a bit more idiomatic:

```
while (p > *value && isspace(p[-1])) --p;
*value_len = p - *value;
```

Also, why are you modifying parse_header() to no longer return negative lengths, but then also modifying the callers to deal with them?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7356#note_95036

[Wei Xie (＠wayne)]

Using > alternative >= is a good idea. I wanted to make sure that the value_len of a space should also be 0, but the code actually guarantees this.

To modify the caller's code, I want the buffer to not be assigned if value_len is 0. Refer to the previous irp_size when value_len is 0, and no modification can be made to the caller's place.

I will modify the code and resubmit.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7356#note_95108

[PATCH 0/1] MR7356: http.sys: Avoid crashes in complete_irp() if http header value is space - closed

[Wei Xie (＠wayne)]

This merge request was closed by Wei Xie.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7356