[Privoxy-commits] [privoxy] 09/11: Fix socks4 and socks4a support under glibc's source fortification

[User Git]

This is an automated email from the git hooks/post-receive script.

git pushed a commit to branch master
in repository privoxy.

commit 19d7684ca10f6c1279568aa19e9a9da2276851f1
Author: Ingo Blechschmidt <iblech at speicherleck.de>
AuthorDate: Sun Nov 5 23:43:54 2023 +0100

    Fix socks4 and socks4a support under glibc's source fortification
    
    With glib'c source fortification, gcc offers the compilation warning
    
        gateway.c: In function 'socks4_connect':
        gateway.c:840:4: warning: 'strlcpy' writing 4988 bytes into a region of size 1 overflows the destination
          840 |    strlcpy(&(c->userid), socks_userid, sizeof(buf) - sizeof(struct socks_op));
              |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        gateway.c:112:9: note: destination object 'userid' of size 1
          112 |    char userid;               /* first byte of userid */
              |         ^~~~~~
    
    resulting in a runtime abort() when using a socks4 or socks4a upstream proxy:
    
        $ privoxy --no-daemon <(echo 'forward-socks4 / 127.0.0.1:9050 .')
        7fef77483740 Info: Privoxy version 3.0.34
        7fef77483740 Info: Program name: privoxy
        *** buffer overflow detected ***: terminated
        rc: line 3: 321835 Aborted    (core dumped) privoxy --no-daemon <(echo 'forward-socks4 / 127.0.0.1:9050 .')
    
    Despite the warning, the strlcpy() call in question is fine: gcc
    misidentifies the size of the destination buffer, estimating to hold
    only a single char while in fact the buffer stretches beyond the end of
    the struct socks_op.
    
    This commit fixes this issue in a way which is in line with the second
    strlcpy() call in the socks4_connect(). Alternatively, we could also
    remove the padding member and promote userid to a trailing flexible array
    member. However, this would necessitate further adjustments because that
    way the size of struct socks_op would change.
    
    The issue was originally reported in the NixOS issue tracker at
    
        https://github.com/NixOS/nixpkgs/issues/265654
    
    prompted by an upgrade of glibc from 2.37-39 to 2.38-0, and the fix is
    joint work with @esclear and @richi235.
---
 gateway.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gateway.c b/gateway.c
index a842c07d..20591426 100644
--- a/gateway.c
+++ b/gateway.c
@@ -837,7 +837,13 @@ static jb_socket socks4_connect(const struct forward_spec *fwd,
 
    /* build a socks request for connection to the web server */
 
-   strlcpy(&(c->userid), socks_userid, sizeof(buf) - sizeof(struct socks_op));
+   /*
+    * The more straightforward &(c->userid) destination pointer can
+    * cause some gcc versions to misidentify the size of the destination
+    * buffer, tripping the runtime check of glibc's source fortification.
+    */
+   strlcpy(buf + offsetof(struct socks_op, userid), socks_userid,
+      sizeof(buf) - sizeof(struct socks_op));
 
    csiz = sizeof(*c) + sizeof(socks_userid) - sizeof(c->userid) - sizeof(c->padding);
 

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.