diff mbox series

[iproute2] ipaddress: strengthen check on 'label' input

Message ID 1524578901-28278-1-git-send-email-ptalbert@redhat.com
State Changes Requested, archived
Delegated to: stephen hemminger
Headers show
Series [iproute2] ipaddress: strengthen check on 'label' input | expand

Commit Message

Patrick Talbert April 24, 2018, 2:08 p.m. UTC
As mentioned in the ip-address man page, an address label must
be equal to the device name or prefixed by the device name
followed by a colon. Currently the only check on this input is
to see if the device name appears at the beginning of the label
string.

This commit adds an additional check to ensure label == dev or
continues with a colon.

Signed-off-by: Patrick Talbert <ptalbert@redhat.com>
---
 ip/ipaddress.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Stephen Hemminger April 26, 2018, 9:45 p.m. UTC | #1
On Tue, 24 Apr 2018 16:08:21 +0200
Patrick Talbert <ptalbert@redhat.com> wrote:

> As mentioned in the ip-address man page, an address label must
> be equal to the device name or prefixed by the device name
> followed by a colon. Currently the only check on this input is
> to see if the device name appears at the beginning of the label
> string.
> 
> This commit adds an additional check to ensure label == dev or
> continues with a colon.
> 
> Signed-off-by: Patrick Talbert <ptalbert@redhat.com>
> ---
>  ip/ipaddress.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index aecc9a1..edcf821 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -2168,9 +2168,14 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
>  		fprintf(stderr, "Not enough information: \"dev\" argument is required.\n");
>  		return -1;
>  	}
> -	if (l && matches(d, l) != 0) {
> -		fprintf(stderr, "\"dev\" (%s) must match \"label\" (%s).\n", d, l);
> -		return -1;
> +	if (l) {
> +		size_t d_len = strlen(d);
> +
> +		if (!(matches(d, l) == 0 && (l[d_len] == '\0' || l[d_len] == ':'))) {

matches is not what you want here. matches does prefix match (ie matches("eth0", "eth") == 0).
Also, what if label is shorter than the device, you would end up dereferencing past
the end of the string!

I think you want something like:

static bool is_valid_label(const char *dev, const char *label)
{
	const char *sep;

	sep = strchr(label, ':');
	if (sep)
		return strncmp(dev, label, sep - label) == 0;
	else
		return strcmp(dev, label) == 0;

}

> +			fprintf(stderr, "\"label\" (%s) must match \"dev\" (%s) or be prefixed by"
> +				" \"dev\" with a colon.\n", l, d);
> +			return -1;
> +		}
>  	}
>  
>  	if (peer_len == 0 && local_len) {
diff mbox series

Patch

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index aecc9a1..edcf821 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -2168,9 +2168,14 @@  static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 		fprintf(stderr, "Not enough information: \"dev\" argument is required.\n");
 		return -1;
 	}
-	if (l && matches(d, l) != 0) {
-		fprintf(stderr, "\"dev\" (%s) must match \"label\" (%s).\n", d, l);
-		return -1;
+	if (l) {
+		size_t d_len = strlen(d);
+
+		if (!(matches(d, l) == 0 && (l[d_len] == '\0' || l[d_len] == ':'))) {
+			fprintf(stderr, "\"label\" (%s) must match \"dev\" (%s) or be prefixed by"
+				" \"dev\" with a colon.\n", l, d);
+			return -1;
+		}
 	}
 
 	if (peer_len == 0 && local_len) {