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

login
register
mail settings
Submitter Thomas Jarosch
Date Dec. 25, 2012, 2:15 p.m.
Message ID <50D9B4FF.9000806@intra2net.com>
Download mbox | patch
Permalink /patch/208108/
State Not Applicable
Headers show

Comments

Thomas Jarosch - Dec. 25, 2012, 2:15 p.m.
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(-)
Pablo Neira - Dec. 26, 2012, 10:11 p.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Patrick McHardy - Dec. 27, 2012, 6:23 a.m.
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.

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thomas Jarosch - Dec. 27, 2012, 9:48 a.m.
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)

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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 )