Summary
When parsing a SOCKS5 response whose address type is a domain name, checkServerReply() reads the domain length from byte 4 but copies the address starting at byte 4 as well. Byte 4 is the length byte, so the parsed host is shifted by one byte and the port offsets are wrong.
Code path
modules/authenticated-socks-module/src/main/java/org/simplejavamail/internal/authenticatedsockssupport/socks5client/SocksCommandSender.java:126-166.
- The domain response layout is
VER, REP, RSV, ATYP, LEN, DOMAIN..., PORT_HI, PORT_LO.
- Lines 162-165 allocate the domain and read the port using offsets that include the length byte.
Steps to reproduce
The offset problem is visible with a minimal SOCKS5 domain response:
domain = b"example.com"
response = bytes([5, 0, 0, 3, len(domain)]) + domain + bytes([0x01, 0xBB])
length = response[4]
current_host = response[4:4 + length]
current_port = response[4 + length:6 + length]
assert current_host != domain
assert current_port != bytes([0x01, 0xBB])
This models the offsets used by the current parser; the length byte is incorrectly treated as the first host byte.
Expected behavior
The host bytes should be copied from offset 5, and the two port bytes should follow the domain at offsets 5 + length and 6 + length.
Actual behavior
The length byte is included in the copied host, and the port is read one byte too early. A domain-name bind response can therefore produce a corrupted log/diagnostic and incorrect port information.
Existing coverage
The checked issue and PR history did not contain an exact report or fix for this domain-response offset.
Suggested fix
Use the protocol offsets explicitly: skip byte 4 when copying the domain and read the port after the domain bytes.
Suggested tests
- A domain response such as
example.com:443.
- One-character and maximum-length domains.
- IPv4 and IPv6 response branches remain unchanged.
- Assert the parsed host and port from exact response bytes.
Submitted with Codex.
Summary
When parsing a SOCKS5 response whose address type is a domain name,
checkServerReply()reads the domain length from byte 4 but copies the address starting at byte 4 as well. Byte 4 is the length byte, so the parsed host is shifted by one byte and the port offsets are wrong.Code path
modules/authenticated-socks-module/src/main/java/org/simplejavamail/internal/authenticatedsockssupport/socks5client/SocksCommandSender.java:126-166.VER, REP, RSV, ATYP, LEN, DOMAIN..., PORT_HI, PORT_LO.Steps to reproduce
The offset problem is visible with a minimal SOCKS5 domain response:
This models the offsets used by the current parser; the length byte is incorrectly treated as the first host byte.
Expected behavior
The host bytes should be copied from offset 5, and the two port bytes should follow the domain at offsets
5 + lengthand6 + length.Actual behavior
The length byte is included in the copied host, and the port is read one byte too early. A domain-name bind response can therefore produce a corrupted log/diagnostic and incorrect port information.
Existing coverage
The checked issue and PR history did not contain an exact report or fix for this domain-response offset.
Suggested fix
Use the protocol offsets explicitly: skip byte 4 when copying the domain and read the port after the domain bytes.
Suggested tests
example.com:443.Submitted with Codex.