Skip to content

Conversation

@ammarfaizi2
Copy link
Contributor

After stress-testing the SOCKS5 proxy server, I found various memory
corruption bugs. Here is the stress-test program:

https://gist.github.com/ammarfaizi2/53324bb7dd8ca7cb73a92b80b62f8f3b

Only the first patch is not related to memory corruption bugs.

1) Fix the wrong batch condition decision.

If there are 1 entries and 5 workers, and nr_sleeping is 5. It will
always batch because nr_entries + 16 is always greater than 5. That's
just wrong.

2) Handle GWP_SOCKS5_ST_CMD_CONNECT in gwp_socks5_conn_handle_data().

The gwp_socks5_conn_handle_data() does not properly handle socks5
data if the CONNECT command comes together with the proxied data.

3) Fix missing buf advance and target.len increment.

gwp_socks5_conn_handle_data() may return -EAGAIN and have in_len
or out_len greater than zero. Make sure to account those values.

4) Fix an invalid handle_socks5_data() call.

The call to handle_socks5_data() must only be done when the
connection is in CONN_STATE_SOCKS5_DATA state.

The following changes since commit 19a2f3db2bdba27a98d77b2f1093ce637155e443:

  Merge branch 'dns' (DNS library integration) (2025-07-17 19:21:22 +0700)

are available in the Git repository at:

  https://github.com/ammarfaizi2/gwproxy.git bug-fix

for you to fetch changes up to 33e63762f946a6180696d4aef7d62362840da486:

  gwproxy: Fix an invalid `handle_socks5_data()` call (2025-07-19 12:32:37 +0700)

----------------------------------------------------------------
Ammar Faizi (4):
      gwproxy/dns: Fix the wrong batch condition decision
      gwproxy/socks5: Handle `GWP_SOCKS5_ST_CMD_CONNECT` in `gwp_socks5_conn_handle_data()`
      gwproxy: Fix missing buf advance and `target.len` increment
      gwproxy: Fix an invalid `handle_socks5_data()` call

 src/gwproxy/dns.c     |  2 +-
 src/gwproxy/gwproxy.c | 18 ++++++++++--------
 src/gwproxy/socks5.c  |  4 ++++
 3 files changed, 15 insertions(+), 9 deletions(-)

If there are 1 entries and 5 workers, and nr_sleeping is 5. It will
always batch because nr_entries + 16 is always greater than 5. That's
just wrong.

Fixes: 317dc79 ("dns: Handle DNS query in batch via `getaddrinfo_a()`")
Signed-off-by: Ammar Faizi <[email protected]>
…n_handle_data()`

The `gwp_socks5_conn_handle_data()` does not properly handle socks5
data if the CONNECT command comes together with the proxied data.

Fixes: 97a3bc1 ("socks5: Initial socks5 modularization")
Signed-off-by: Ammar Faizi <[email protected]>
`gwp_socks5_conn_handle_data()` may return `-EAGAIN` and have `in_len`
or `out_len` greater than zero. Make sure to account those values.

Fixes: 3fad620 ("gwproxy: socks5: Use the socks5 library")
Signed-off-by: Ammar Faizi <[email protected]>
The call to `handle_socks5_data()` must only be done when the
connection is in `CONN_STATE_SOCKS5_DATA` state.

Fixes: 3fad620 ("gwproxy: socks5: Use the socks5 library")
Signed-off-by: Ammar Faizi <[email protected]>
@ammarfaizi2 ammarfaizi2 merged commit 052f8a2 into master Jul 19, 2025
1 check passed
ammarfaizi2 added a commit that referenced this pull request Jul 19, 2025
After stress-testing the SOCKS5 proxy server, I found various memory
corruption bugs. Here is the stress-test program:

  https://gist.github.com/ammarfaizi2/53324bb7dd8ca7cb73a92b80b62f8f3b

Only the first patch is not related to memory corruption bugs.

* bug-fix:
  gwproxy: Fix an invalid `handle_socks5_data()` call
  gwproxy: Fix missing buf advance and `target.len` increment
  gwproxy/socks5: Handle `GWP_SOCKS5_ST_CMD_CONNECT` in `gwp_socks5_conn_handle_data()`
  gwproxy/dns: Fix the wrong batch condition decision

Link: #4
Signed-off-by: Ammar Faizi <[email protected]>
ammarfaizi2 added a commit that referenced this pull request Jul 19, 2025
After stress-testing the SOCKS5 proxy server, I found various memory
corruption bugs. Here is the stress-test program:

  https://gist.github.com/ammarfaizi2/53324bb7dd8ca7cb73a92b80b62f8f3b

Only the first patch is not related to memory corruption bugs.

* bug-fix:
  gwproxy: Fix an invalid `handle_socks5_data()` call
  gwproxy: Fix missing buf advance and `target.len` increment
  gwproxy/socks5: Handle `GWP_SOCKS5_ST_CMD_CONNECT` in `gwp_socks5_conn_handle_data()`
  gwproxy/dns: Fix the wrong batch condition decision

Link: #4
Signed-off-by: Ammar Faizi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants