diff mbox

[U-Boot,RFC,3/8] lib: net_utils: make string_to_ip stricter

Message ID 1444635794-16285-4-git-send-email-judge.packham@gmail.com
State RFC
Delegated to: Joe Hershberger
Headers show

Commit Message

Chris Packham Oct. 12, 2015, 7:43 a.m. UTC
Previously values greater than 255 were implicitly truncated. Add some
stricter checking to reject addresses with components >255.

With the input "1234192.168.1.1" the old behaviour would truncate the
address to 192.168.1.1. New behaviour rejects the string outright and
returns 0.0.0.0, which for the purposes of IP addresses can be
considered an error.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---

 lib/net_utils.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Joe Hershberger Nov. 2, 2015, 8:42 p.m. UTC | #1
Hi Chris,

On Mon, Oct 12, 2015 at 2:43 AM, Chris Packham <judge.packham@gmail.com> wrote:
> Previously values greater than 255 were implicitly truncated. Add some
> stricter checking to reject addresses with components >255.
>
> With the input "1234192.168.1.1" the old behaviour would truncate the
> address to 192.168.1.1. New behaviour rejects the string outright and
> returns 0.0.0.0, which for the purposes of IP addresses can be
> considered an error.
>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---

Acked-by: Joe Hershberger
Chris Packham Nov. 9, 2015, 2:49 a.m. UTC | #2
On Mon, Oct 12, 2015 at 8:43 PM, Chris Packham <judge.packham@gmail.com> wrote:
> Previously values greater than 255 were implicitly truncated. Add some
> stricter checking to reject addresses with components >255.
>
> With the input "1234192.168.1.1" the old behaviour would truncate the
> address to 192.168.1.1. New behaviour rejects the string outright and
> returns 0.0.0.0, which for the purposes of IP addresses can be
> considered an error.
>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
>
>  lib/net_utils.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/lib/net_utils.c b/lib/net_utils.c
> index cfae842..0fca54d 100644
> --- a/lib/net_utils.c
> +++ b/lib/net_utils.c
> @@ -24,10 +24,19 @@ struct in_addr string_to_ip(const char *s)
>
>         for (addr.s_addr = 0, i = 0; i < 4; ++i) {
>                 ulong val = s ? simple_strtoul(s, &e, 10) : 0;
> +               if (val > 255) {
> +                       addr.s_addr = 0;
> +                       return addr;
> +               }
>                 addr.s_addr <<= 8;
>                 addr.s_addr |= (val & 0xFF);
> -               if (s) {
> -                       s = (*e) ? e+1 : e;
> +               if (*e == '.') {
> +                       s = e + 1;
> +               } else if (*e == '\0') {
> +                       break;
> +               } else {
> +                       addr.s_addr = 0;
> +                       return addr;

One unintended side effect of this is that is breaks parsing of things
like 'tftpboot 192.168.1.1:zImage'. I'll fix that in the next round.

>                 }
>         }
>
> --
> 2.5.3
>
diff mbox

Patch

diff --git a/lib/net_utils.c b/lib/net_utils.c
index cfae842..0fca54d 100644
--- a/lib/net_utils.c
+++ b/lib/net_utils.c
@@ -24,10 +24,19 @@  struct in_addr string_to_ip(const char *s)
 
 	for (addr.s_addr = 0, i = 0; i < 4; ++i) {
 		ulong val = s ? simple_strtoul(s, &e, 10) : 0;
+		if (val > 255) {
+			addr.s_addr = 0;
+			return addr;
+		}
 		addr.s_addr <<= 8;
 		addr.s_addr |= (val & 0xFF);
-		if (s) {
-			s = (*e) ? e+1 : e;
+		if (*e == '.') {
+			s = e + 1;
+		} else if (*e == '\0') {
+			break;
+		} else {
+			addr.s_addr = 0;
+			return addr;
 		}
 	}