[PATCH] smackfs: reject out-of-range IPv4 octets in netlabel writes

[PATCH] smackfs: reject out-of-range IPv4 octets in netlabel writes

[Hongjian Dai]

smk_write_net4addr() parses the address with %hhd, which silently
wraps values over 255 (e.g. 300 becomes 44) and accepts them. The
wrong address is then stored and passed to netlbl_cfg_unlbl_static_add(),
so inbound packets get mislabeled and policy can be bypassed or
connections wrongly denied by a privileged misconfiguration.

Parse into unsigned ints like the IPv6 path does and reject any
octet above 255 with -EINVAL, mirroring the existing scanned[i] >
0xffff check in smk_write_net6addr().

Fixes: 6d3dc07cbb1e ("smack: Add support for unlabeled network hosts and networks")
Signed-off-by: Hongjian Dai <daihongjian@kylinsec.com.cn>
---
 security/smack/smackfs.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index a72bc7fabea9..845660cc3a7b 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -1169,6 +1169,7 @@ static ssize_t smk_write_net4addr(struct file *file, const char __user *buf,
 	struct in_addr mask;
 	unsigned int m;
 	unsigned int masks;
+	unsigned int octet[4];
 	int found;
 	u32 mask_bits = (1<<31);
 	__be32 nsa;
@@ -1198,17 +1199,26 @@ static ssize_t smk_write_net4addr(struct file *file, const char __user *buf,
 		goto free_data_out;
 	}
 
-	rc = sscanf(data, "%hhd.%hhd.%hhd.%hhd/%u %s",
-		&host[0], &host[1], &host[2], &host[3], &masks, smack);
+	rc = sscanf(data, "%u.%u.%u.%u/%u %s",
+		&octet[0], &octet[1], &octet[2], &octet[3], &masks, smack);
 	if (rc != 6) {
-		rc = sscanf(data, "%hhd.%hhd.%hhd.%hhd %s",
-			&host[0], &host[1], &host[2], &host[3], smack);
+		rc = sscanf(data, "%u.%u.%u.%u %s",
+			&octet[0], &octet[1], &octet[2], &octet[3], smack);
 		if (rc != 5) {
 			rc = -EINVAL;
 			goto free_out;
 		}
 		masks = 32;
 	}
+	if (octet[0] > 255 || octet[1] > 255 || octet[2] > 255 ||
+	    octet[3] > 255) {
+		rc = -EINVAL;
+		goto free_out;
+	}
+	host[0] = octet[0];
+	host[1] = octet[1];
+	host[2] = octet[2];
+	host[3] = octet[3];
 	if (masks > BEBITS) {
 		rc = -EINVAL;
 		goto free_out;
-- 
2.43.0

Re: [PATCH] smackfs: reject out-of-range IPv4 octets in netlabel writes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] smackfs: out-of-bounds IPv4 octet validation bypassed via integer wrap-around
--

commit 13135cde4c7fdf9b39cec7b05019b4e50745e1ad
Author: Hongjian Dai <daihongjian@kylinsec.com.cn>

smackfs: reject out-of-range IPv4 octets in netlabel writes

This commit attempts to fix a misconfiguration vulnerability in
smk_write_net4addr() where out-of-range IPv4 octets were silently wrapped by
sscanf with %hhd. It changes the sscanf format to %u and adds explicit
bounds checking before storing the address.

> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 6e62dcb36f74f..5d8bddffbabd3 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c

[ ... ]

> @@ -1189,17 +1190,26 @@ static ssize_t smk_write_net4addr(struct file *file, const char __user *buf,
>  		goto free_data_out;
>  	}
>  
> -	rc = sscanf(data, "%hhd.%hhd.%hhd.%hhd/%u %s",
> -		&host[0], &host[1], &host[2], &host[3], &masks, smack);
> +	rc = sscanf(data, "%u.%u.%u.%u/%u %s",
> +		&octet[0], &octet[1], &octet[2], &octet[3], &masks, smack);
>  	if (rc != 6) {
> -		rc = sscanf(data, "%hhd.%hhd.%hhd.%hhd %s",
> -			&host[0], &host[1], &host[2], &host[3], smack);
> +		rc = sscanf(data, "%u.%u.%u.%u %s",
> +			&octet[0], &octet[1], &octet[2], &octet[3], smack);
>  		if (rc != 5) {
>  			rc = -EINVAL;
>  			goto free_out;
>  		}
>  		masks = 32;
>  	}
> +	if (octet[0] > 255 || octet[1] > 255 || octet[2] > 255 ||
> +	    octet[3] > 255) {

[Severity: Medium]
Does this still silently wrap if an input value is >= 2^32?

The kernel's sscanf implementation for %u parses into a 64-bit unsigned long
long and truncates it to a 32-bit unsigned int.

Input values like 4294967296 are silently truncated to values <= 255, which
would completely bypass this new validation check and reproduce the exact
silent-wrapping misconfiguration this patch intends to fix.

> +		rc = -EINVAL;
> +		goto free_out;
> +	}
> +	host[0] = octet[0];
> +	host[1] = octet[1];
> +	host[2] = octet[2];
> +	host[3] = octet[3];
>  	if (masks > BEBITS) {
>  		rc = -EINVAL;
>  		goto free_out;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/FFA41AC93858A96A+20260921014555.20351-1-daihongjian@kylinsec.com.cn?part=1