| Message ID | 41dee8ac-ab8f-4a12-94b5-81b2a9852115o@googlegroups.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [meta-swupdate,libubootenv] The variable type attributes i and m are now checked for their valid format if a new variable value is assigned | expand |
Hi Uwe, On 05.06.20 16:10, Uwe Schuster wrote: > From 2de70f78b3f8121b64b1eca52f21ff985ba555e5 Mon Sep 17 00:00:00 2001 > From: Uwe Schuster <uwe.schuster@woodward.com> > Date: Thu, 4 Jun 2020 17:15:08 +0200 > Subject: [PATCH] The variable type attributes i and m are now checked for > their valid format if a new variable value is assigned. > > Attribute modifier 'i' means an IP address and 'm' a MAC address string. > Both are considered valid in the same simplified way as in U-Boot source > code itself, despite the fact that also only 192.1 would be a valid IP > address. > > Signed-off-by: Uwe Schuster <uwe.schuster@woodward.com> > --- > src/uboot_env.c | 63 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 62 insertions(+), 1 deletion(-) > > diff --git a/src/uboot_env.c b/src/uboot_env.c > index 1d9b0f9..ff46dbb 100644 > --- a/src/uboot_env.c > +++ b/src/uboot_env.c > @@ -116,6 +116,62 @@ static char access_tostring(access_attribute a) > return 'a'; > } > > + > +/* attribution to https://stackoverflow.com/questions/4792035 > + and https://stackoverflow.com/users/454406/daniel-gehriger > + */ Agree that any reference to external code must be described. But here I cannot know under which license this code was published, even if put on StackOverflow. It could be even closed source and I get big legal issues. Code is simple, and you can get in multiple ways without copying it. You could even do a single sscanf(eth_addr, "%02X:%02X:%02X:%02X:%02X:%02X"). > +static int validate_ethaddr_str(const char *eth_addr) > +{ > + int i = 0; > + int s = 0; > + > + while (eth_addr && *eth_addr) { > + if (isxdigit(*eth_addr)) { > + i++; > + } > + else if (*eth_addr == ':') { > + if (i == 0 || i / 2 - 1 != s) > + break; > + ++s; > + } > + else > + s = -1; > + > + ++eth_addr; > + } > + > + if (i == 12 && (s == 5)) > + return 0; > + else > + return -1; > +} > + > +/* attribution to https://stackoverflow.com/questions/791982 > + and https://stackoverflow.com/users/952747/deepmax > + */ > +static int validate_ipaddr_str(const char *ip_addr) > +{ But there is a inet_ntop() function in libc, isn't it ? > + int len = strlen(ip_addr); > + > + if (len < 7 || len > 15) > + return -1; > + > + char tail[16]; > + tail[0] = 0; > + > + unsigned int d[4]; > + int c = sscanf(ip_addr, "%3u.%3u.%3u.%3u%s", &d[0], &d[1], &d[2], > &d[3], tail); > + > + if (c != 4 || tail[0]) > + return -1; > + > + for (int i = 0; i < 4; i++) > + if (d[i] > 255) > + return -1; > + > + return 0; > +} > + > static struct var_entry *__libuboot_get_env(struct vars *envs, const > char *varname) > { > struct var_entry *entry; > @@ -1265,13 +1321,18 @@ static bool libuboot_validate_flags(struct > var_entry *entry, const char *value) > } > break; > case TYPE_ATTR_BOOL: > - ok_access = (value[0] == '1' || value[0] == 'y' || value[0] == > 't' || > + ok_type = (value[0] == '1' || value[0] == 'y' || value[0] == 't' || This seems a *bug*, please fix it in a separate patch - thanks ! > value[0] == 'Y' || value[0] == 'T' || > value[0] == '0' || value[0] == 'n' || value[0] == 'f' || > value[0] == 'N' || value[0] == 'F') && (strlen(value) != 1); > break; > case TYPE_ATTR_IP: > + if ( 0 != validate_ipaddr_str(value) ) No Yoda > + ok_type = false; > + break; > case TYPE_ATTR_MAC: > + if ( 0 != validate_ethaddr_str(value) ) > + ok_type = false; > break; > } > #if !defined(NDEBUG) > Best regards, Stefano Babic
diff --git a/src/uboot_env.c b/src/uboot_env.c index 1d9b0f9..ff46dbb 100644 --- a/src/uboot_env.c +++ b/src/uboot_env.c @@ -116,6 +116,62 @@ static char access_tostring(access_attribute a) return 'a'; } + +/* attribution to https://stackoverflow.com/questions/4792035 + and https://stackoverflow.com/users/454406/daniel-gehriger + */ +static int validate_ethaddr_str(const char *eth_addr) +{ + int i = 0; + int s = 0; + + while (eth_addr && *eth_addr) { + if (isxdigit(*eth_addr)) { + i++; + } + else if (*eth_addr == ':') { + if (i == 0 || i / 2 - 1 != s) + break; + ++s; + } + else + s = -1; + + ++eth_addr; + } + + if (i == 12 && (s == 5)) + return 0; + else + return -1; +} + +/* attribution to https://stackoverflow.com/questions/791982 + and https://stackoverflow.com/users/952747/deepmax + */ +static int validate_ipaddr_str(const char *ip_addr) +{ + int len = strlen(ip_addr); + + if (len < 7 || len > 15) + return -1; + + char tail[16]; + tail[0] = 0; + + unsigned int d[4]; + int c = sscanf(ip_addr, "%3u.%3u.%3u.%3u%s", &d[0], &d[1], &d[2], &d[3], tail); + + if (c != 4 || tail[0]) + return -1; + + for (int i = 0; i < 4; i++) + if (d[i] > 255) + return -1; + + return 0; +} + static struct var_entry *__libuboot_get_env(struct vars *envs, const char *varname)