[libnl-nft PATCH] Fix out of bounds buffer access in rtnl_netem_set_delay_distribution()

[libnl-nft PATCH] Fix out of bounds buffer access in rtnl_netem_set_delay_distribution()

[Thomas Jarosch]

sizeof(test_path) returns the array size
and not the number of array elements.

Detected by cppcheck

Signed-off-by: Thomas Jarosch <thomas.jarosch@intra2net.com>
---
 lib/route/sch/netem.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/route/sch/netem.c b/lib/route/sch/netem.c
index 18878a7..5894fd9 100644
--- a/lib/route/sch/netem.c
+++ b/lib/route/sch/netem.c
@@ -861,7 +861,7 @@ int rtnl_netem_set_delay_distribution(struct rtnl_qdisc *qdisc, const char *dist
 		return -NLE_NOMEM;
 		
 	FILE *f = NULL;
-	int i, n = 0;
+	int i = 0, n = 0;
 	size_t len = 2048;
 	char *line;
 	char name[NAME_MAX];
@@ -873,11 +873,12 @@ int rtnl_netem_set_delay_distribution(struct rtnl_qdisc *qdisc, const char *dist
 		strcpy(dist_suffix, "");
 	
 	/* Check several locations for the dist file */
-	char *test_path[] = { "", "./", "/usr/lib/tc/", "/usr/local/lib/tc/" };
+	char *test_path[] = { "", "./", "/usr/lib/tc/", "/usr/local/lib/tc/", NULL };
 	
-	for (i = 0; i < sizeof(test_path) && f == NULL; i++) {
+	while(f == NULL && test_path[i] != NULL) {
 		snprintf(name, NAME_MAX, "%s%s%s", test_path[i], dist_type, dist_suffix);
 		f = fopen(name, "r");
+		++i;
 	}
 	
 	if ( f == NULL )
-- 
1.7.11.7

Re: [libnl-nft PATCH] Fix out of bounds buffer access in rtnl_netem_set_delay_distribution()

[Pablo Neira Ayuso]

Hi Thomas!

On Tue, Dec 25, 2012 at 03:15:27PM +0100, Thomas Jarosch wrote:
> sizeof(test_path) returns the array size
> and not the number of array elements.
> 
> Detected by cppcheck
> 
> Signed-off-by: Thomas Jarosch <thomas.jarosch@intra2net.com>
> ---
>  lib/route/sch/netem.c | 7 ++++---

These two fixes you sent belong to non-netfilter libnl subsystems. You
should better go to libnl mailing and post them there.

Re: [libnl-nft PATCH] Fix out of bounds buffer access in rtnl_netem_set_delay_distribution()

[Patrick McHardy]

On Tue, 25 Dec 2012, Thomas Jarosch wrote:

> sizeof(test_path) returns the array size
> and not the number of array elements.
>
> Detected by cppcheck

Is this already fixed in libnl upstream? I'm considering just doing a full 
merge.

Re: [libnl-nft PATCH] Fix out of bounds buffer access in rtnl_netem_set_delay_distribution()

[Thomas Jarosch]

On Thursday, 27. December 2012 07:23:12 Patrick McHardy wrote:
> On Tue, 25 Dec 2012, Thomas Jarosch wrote:
> > sizeof(test_path) returns the array size
> > and not the number of array elements.
> > 
> > Detected by cppcheck
> 
> Is this already fixed in libnl upstream? I'm considering just doing a full
> merge.

This one has been fixed upstream.

(I wasn't even aware there's an upstream... I just tested
some cppcheck changes on the git trees I had on disc)