Hi,
Sorry, two corrections to my previous message:
- The attached 2/2 patch was damaged while sending. A context line in the
second hunk lost a "*", so it does not apply. The correct file is attached
here. 1/2 is fine.
- My mail client turned the addresses in the result table into links. Here
it is again with ports only (src 1.2.3.4, dst 5.6.7.8 in every case):
ihl=5 hdr_len=40: src_port=4660 dst_port=80 (normal)
ihl=0 hdr_len=40: src_port=16384 dst_port=(empty) (ports from the IP header)
ihl=15 hdr_len=40: src_port=25658 dst_port=8345 (bytes of the previous
connection's request)
ihl=5 hdr_len=20: src_port=18245 dst_port=21536 ("GE" "T " from the HTTP
payload)
Thanks,
Martin Mirchev
On Tue, Oct 27, 2026 03:49 PM, Martin Mirchev <[email protected]>
wrote:
> Hi,
>
> Here are two small fixes for conn_recv_netscaler_cip(), against current
> master (3e758361c). The patches are attached (git format-patch). The first
> one is the fix described in GitHub issue #3549.
>
> 1/2 BUG/MINOR: connection: check the NetScaler CIP header length against
> received data
> hdr_len is only compared with the whole peeked data, which includes
> the 8- or 12-byte CIP prefix. The header is then consumed with a single
> recv() of prefix + hdr_len bytes into the trash chunk. With hdr_len
> close to tune.bufsize, that recv() writes up to 12 bytes past the end
> of the trash buffer. The patch requires the announced header to fit in
> the data received after the prefix.
>
> 2/2 BUG/MINOR: connection: check the IP header length in NetScaler CIP
> The TCP ports are taken at line + ip_hl * 4 without checking that
> ip_hl >= 5, or that hdr_len covers them. The same is true for IPv6 when
> hdr_len < 44. All reads stay inside the trash buffer, so the only
> effect is wrong src_port/dst_port values. With a release build of
> master and nbthread 1, a frontend returning
> src=%[src]:%[src_port] dst=%[dst]:%[dst_port] gives:
>
> ihl=5 hdr_len=40: src=https://www.google.com/url?q=http://1.2.3.4:4660&;
> source=gmail&ust=1793173788945000&sa=E dst=https://www.google.com/
> url?q=http://5.6.7.8:80&source=gmail&ust=1793173788945000&sa=E (normal)
> ihl=0 hdr_len=40: src=https://www.google.com/url?q=http://1.2.3.4:16384&;
> source=gmail&ust=1793173788945000&sa=E dst=https://www.google.com/
> url?q=http://5.6.7.8&source=gmail&ust=1793173788945000&sa=E: (ports from
> the IP header)
> ihl=15 hdr_len=40: src=https://www.google.com/url?q=http://1.2.3.4:25658&;
> source=gmail&ust=1793173788945000&sa=E dst=https://www.google.com/
> url?q=http://5.6.7.8:8345&source=gmail&ust=1793173788945000&sa=E (bytes
> of the previous connection's request)
> ihl=5 hdr_len=20: src=https://www.google.com/url?q=http://1.2.3.4:18245&;
> source=gmail&ust=1793173788945000&sa=E dst=https://www.google.com/
> url?q=http://5.6.7.8:21536&source=gmail&ust=1793173788945000&sa=E ("GE"
> "T " from the HTTP payload)
>
> With the patch, these malformed headers are rejected with
> CO_ER_CIP_BAD_PROTO, and the valid case behaves as before. This patch
> relies on 1/2, which guarantees that hdr_len bytes were actually
> received.
>
> CIP is only accepted from trusted peers, so neither patch fixes a
> security issue. They are robustness fixes. reg-tests/connection/
> netscaler_cip.vtc passes with both patches. Both should be backported to
> all stable branches.
>
> This issue was found by an LLM-Enhanced Static Analysis Engine and
> verified manually.
>
> Thanks,
> Martin Mirchev
>
From 5f4e8988e0caf18e9b5bfa33401f5f7c0eb314f0 Mon Sep 17 00:00:00 2001
From: Martin Mirchev <[email protected]>
Date: Tue, 27 Oct 2026 14:17:35 +0800
Subject: [PATCH 2/2] BUG/MINOR: connection: check the IP header length in
NetScaler CIP
conn_recv_netscaler_cip() takes the TCP ports at ip_hl * 4 bytes after the
start of the IPv4 header without checking ip_hl nor that the announced
header length covers them:
- with ip_hl < 5 the ports are taken from the IPv4 header itself (e.g.
ip_hl=0 gives source port 16384 from the version/IHL byte);
- with a header length shorter than ip_hl * 4 + 4 (e.g. ip_hl=15 and a
40-byte header) they are taken from the payload or from bytes past the
received data, which are leftovers of the previous use of the trash
buffer, possibly from another connection.
The same applies to IPv6 with a header length shorter than 44 bytes. The
reads stay inside the trash buffer, so this is not a memory safety issue,
but the session addresses end up with wrong ports. Let's reject such
headers with CO_ER_CIP_BAD_PROTO.
This should be backported to all stable versions.
---
src/connection.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/connection.c b/src/connection.c
index 52fe22038..051efc22a 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -1784,6 +1784,14 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag)
conn->err_code = CO_ER_CIP_BAD_PROTO;
goto fail;
}
+ else if (hdr_ip4->ip_hl < 5 ||
+ hdr_len < hdr_ip4->ip_hl * 4 + sizeof(struct my_tcphdr)) {
+ /* The IPv4 header length is invalid or the announced
+ * header is too short to contain the TCP ports.
+ */
+ conn->err_code = CO_ER_CIP_BAD_PROTO;
+ goto fail;
+ }
hdr_tcp = (struct my_tcphdr *)(line + (hdr_ip4->ip_hl * 4));
@@ -1815,6 +1823,11 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag)
conn->err_code = CO_ER_CIP_BAD_PROTO;
goto fail;
}
+ else if (hdr_len < sizeof(struct ip6_hdr) + sizeof(struct my_tcphdr)) {
+ /* The announced header is too short to contain the TCP ports */
+ conn->err_code = CO_ER_CIP_BAD_PROTO;
+ goto fail;
+ }
hdr_tcp = (struct my_tcphdr *)(line + sizeof(struct ip6_hdr));
--
2.50.1
