[PATCH] selftests: mptcp: mptcp_diag: fix stack buffer overflow in get_subflow_info()

[PATCH] selftests: mptcp: mptcp_diag: fix stack buffer overflow in get_subflow_info()

[Jiangshan Yi]

get_subflow_info() parses the subflow address string with:

	char saddr[64], daddr[64];

	ret = sscanf(subflow_addrs, "%[^:]:%d %[^:]:%d",
		     saddr, &sport, daddr, &dport);

The subflow_addrs buffer holds up to 1024 bytes and is taken directly
from the command line ("-c" argument). The "%[^:]" conversions have no
maximum field width, so if the address substring before the ':' exceeds
63 bytes, sscanf() writes past the end of the 64-byte saddr/daddr stack
buffers. This overflows the stack, corrupting adjacent stack data such
as the saved return address, and can crash the tool or lead to
out-of-bounds writes controlled by user-supplied input.

Bound both string conversions to the destination buffer size by adding
an explicit maximum field width of 63 (leaving room for the terminating
NUL), so at most 63 bytes are written into each 64-byte buffer:

	ret = sscanf(subflow_addrs, "%63[^:]:%d %63[^:]:%d",
		     saddr, &sport, daddr, &dport);

Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_diag.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
index 5e222ba977e4..02ac93f794fe 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
@@ -377,7 +377,7 @@ static void get_subflow_info(char *subflow_addrs)
 	int ret;
 	int fd;
 
-	ret = sscanf(subflow_addrs, "%[^:]:%d %[^:]:%d", saddr, &sport, daddr, &dport);
+	ret = sscanf(subflow_addrs, "%63[^:]:%d %63[^:]:%d", saddr, &sport, daddr, &dport);
 	if (ret != 4)
 		die_perror("IP PORT Pairs has style problems!");
 
-- 
2.25.1

Re: [PATCH] selftests: mptcp: mptcp_diag: fix stack buffer overflow in get_subflow_info()

[Geliang Tang]

Hi Jiangshan,

On Wed, 2026-07-01 at 18:38 +0800, Jiangshan Yi wrote:
> get_subflow_info() parses the subflow address string with:
> 
> 	char saddr[64], daddr[64];
> 
> 	ret = sscanf(subflow_addrs, "%[^:]:%d %[^:]:%d",
> 		     saddr, &sport, daddr, &dport);
> 
> The subflow_addrs buffer holds up to 1024 bytes and is taken directly
> from the command line ("-c" argument). The "%[^:]" conversions have
> no
> maximum field width, so if the address substring before the ':'
> exceeds
> 63 bytes, sscanf() writes past the end of the 64-byte saddr/daddr
> stack
> buffers. This overflows the stack, corrupting adjacent stack data
> such
> as the saved return address, and can crash the tool or lead to
> out-of-bounds writes controlled by user-supplied input.
> 
> Bound both string conversions to the destination buffer size by
> adding
> an explicit maximum field width of 63 (leaving room for the
> terminating
> NUL), so at most 63 bytes are written into each 64-byte buffer:
> 
> 	ret = sscanf(subflow_addrs, "%63[^:]:%d %63[^:]:%d",
> 		     saddr, &sport, daddr, &dport);
> 
> Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_diag.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> index 5e222ba977e4..02ac93f794fe 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> @@ -377,7 +377,7 @@ static void get_subflow_info(char *subflow_addrs)
>  	int ret;
>  	int fd;
>  
> -	ret = sscanf(subflow_addrs, "%[^:]:%d %[^:]:%d", saddr,
> &sport, daddr, &dport);
> +	ret = sscanf(subflow_addrs, "%63[^:]:%d %63[^:]:%d", saddr,
> &sport, daddr, &dport);

Thanks for this patch. MPTCP CI complains:

WARNING: line length of 91 exceeds 80 columns
#44: FILE: tools/testing/selftests/net/mptcp/mptcp_diag.c:380:

Also, for the subject prefix, we usually use "selftests: mptcp: diag:"
instead of "selftests: mptcp: mptcp_diag:". Please consider updating it
if you spin a v2.

Thanks,
-Geliang

>  	if (ret != 4)
>  		die_perror("IP PORT Pairs has style problems!");
>