Bug#511720: [PATCH iproute2] ss: avoid passing negative numbers to malloc

Bug#511720: [PATCH iproute2] ss: avoid passing negative numbers to malloc

[Andreas Henriksson]

Example:

$ ss state established \( sport = :4060  or sport = :4061 or sport = :4062  or sport = :4063 or sport = :4064  or sport = :4065 or sport = :4066  or sport = :4067 \)  > /dev/null
Aborted

In the example above ssfilter_bytecompile(...) will return (int)136.
char l1 = 136; means -120 which will result in a negative number
being passed to malloc at misc/ss.c:913.

Simply declare l1 and l2 as intergers to avoid the char overflow.

This is one of the issues originally reported in http://bugs.debian.org/511720

Reported-by: Andreas Schuldei <andreas@debian.org>
Signed-off-by: Andreas Henriksson <andreas@fatal.se>
---
 misc/ss.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/misc/ss.c b/misc/ss.c
index c0369f1..db3a3a4 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -907,7 +907,8 @@ static int ssfilter_bytecompile(struct ssfilter *f, char **bytecode)
 	}
 		case SSF_OR:
 	{
-		char *a1, *a2, *a, l1, l2;
+		char *a1, *a2, *a;
+		int l1, l2;
 		l1 = ssfilter_bytecompile(f->pred, &a1);
 		l2 = ssfilter_bytecompile(f->post, &a2);
 		if (!(a = malloc(l1+l2+4))) abort();
-- 
1.8.4.3

Re: [PATCH iproute2] ss: avoid passing negative numbers to malloc

[Eric Dumazet]

On Tue, 2013-11-12 at 19:52 +0100, Andreas Henriksson wrote:
> Example:
> 
> $ ss state established \( sport = :4060  or sport = :4061 or sport = :4062  or sport = :4063 or sport = :4064  or sport = :4065 or sport = :4066  or sport = :4067 \)  > /dev/null
> Aborted
> 
> In the example above ssfilter_bytecompile(...) will return (int)136.
> char l1 = 136; means -120 which will result in a negative number
> being passed to malloc at misc/ss.c:913.
> 
> Simply declare l1 and l2 as intergers to avoid the char overflow.
> 
> This is one of the issues originally reported in http://bugs.debian.org/511720
> 
> Reported-by: Andreas Schuldei <andreas@debian.org>
> Signed-off-by: Andreas Henriksson <andreas@fatal.se>
> ---
>  misc/ss.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/misc/ss.c b/misc/ss.c
> index c0369f1..db3a3a4 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -907,7 +907,8 @@ static int ssfilter_bytecompile(struct ssfilter *f, char **bytecode)
>  	}
>  		case SSF_OR:
>  	{
> -		char *a1, *a2, *a, l1, l2;
> +		char *a1, *a2, *a;
> +		int l1, l2;
>  		l1 = ssfilter_bytecompile(f->pred, &a1);
>  		l2 = ssfilter_bytecompile(f->post, &a2);
>  		if (!(a = malloc(l1+l2+4))) abort();

Please fix the other cases (SSF_NOT and SSF_AND) ?

Thanks !

[PATCH iproute2 v2] ss: avoid passing negative numbers to malloc

[Andreas Henriksson]

Example:

$ ss state established \( sport = :4060  or sport = :4061 or sport = :4062  or sport = :4063 or sport = :4064  or sport = :4065 or sport = :4066  or sport = :4067 \)  > /dev/null
Aborted

In the example above ssfilter_bytecompile(...) will return (int)136.
char l1 = 136; means -120 which will result in a negative number
being passed to malloc at misc/ss.c:913.

Simply declare l1 and l2 as integers to avoid the char overflow.

This is one of the issues originally reported in http://bugs.debian.org/511720

Fix the same problem in other code paths as well (thanks to Eric Dumazet).

Reported-by: Andreas Schuldei <andreas@debian.org>
Signed-off-by: Andreas Henriksson <andreas@fatal.se>
---
 misc/ss.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

v2 fixes the same problem in other code paths (AND, NOT).
Pointed out by Eric Dumazet.


diff --git a/misc/ss.c b/misc/ss.c
index c0369f1..6f38ae7 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -894,7 +894,8 @@ static int ssfilter_bytecompile(struct ssfilter *f, char **bytecode)
 
 		case SSF_AND:
 	{
-		char *a1, *a2, *a, l1, l2;
+		char *a1, *a2, *a;
+		int l1, l2;
 		l1 = ssfilter_bytecompile(f->pred, &a1);
 		l2 = ssfilter_bytecompile(f->post, &a2);
 		if (!(a = malloc(l1+l2))) abort();
@@ -907,7 +908,8 @@ static int ssfilter_bytecompile(struct ssfilter *f, char **bytecode)
 	}
 		case SSF_OR:
 	{
-		char *a1, *a2, *a, l1, l2;
+		char *a1, *a2, *a;
+		int l1, l2;
 		l1 = ssfilter_bytecompile(f->pred, &a1);
 		l2 = ssfilter_bytecompile(f->post, &a2);
 		if (!(a = malloc(l1+l2+4))) abort();
@@ -920,7 +922,8 @@ static int ssfilter_bytecompile(struct ssfilter *f, char **bytecode)
 	}
 		case SSF_NOT:
 	{
-		char *a1, *a, l1;
+		char *a1, *a;
+		int l1;
 		l1 = ssfilter_bytecompile(f->pred, &a1);
 		if (!(a = malloc(l1+4))) abort();
 		memcpy(a, a1, l1);
-- 
1.8.4.3

Re: [PATCH iproute2 v2] ss: avoid passing negative numbers to malloc

[Eric Dumazet]

On Wed, 2013-11-13 at 09:46 +0100, Andreas Henriksson wrote:
> Example:
> 
> $ ss state established \( sport = :4060  or sport = :4061 or sport = :4062  or sport = :4063 or sport = :4064  or sport = :4065 or sport = :4066  or sport = :4067 \)  > /dev/null
> Aborted
> 
> In the example above ssfilter_bytecompile(...) will return (int)136.
> char l1 = 136; means -120 which will result in a negative number
> being passed to malloc at misc/ss.c:913.
> 
> Simply declare l1 and l2 as integers to avoid the char overflow.
> 
> This is one of the issues originally reported in http://bugs.debian.org/511720
> 
> Fix the same problem in other code paths as well (thanks to Eric Dumazet).
> 
> Reported-by: Andreas Schuldei <andreas@debian.org>
> Signed-off-by: Andreas Henriksson <andreas@fatal.se>

Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks !