Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 15 Jun 2009 21:03:26 +0000 (UTC)
From:      Stanislav Sedov <stas@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org
Subject:   svn commit: r194265 - in stable/7/contrib/ipfilter: . lib
Message-ID:  <200906152103.n5FL3Q3Z001871@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: stas
Date: Mon Jun 15 21:03:25 2009
New Revision: 194265
URL: http://svn.freebsd.org/changeset/base/194265

Log:
  MFC r193043:
    - Prevent buffer overflow in IPFilter's load_http function used to load
      ipfilter tables via http by the user-level ippool utility. Previously
      the 1024-byte buffer used to store a http request coudld easily overflow
      if the length of the hostname part of the url passes exceeded 496 bytes.
    - Use snprintf to prevent possieble buffer overflows in future.
    - Do not try to close the descriptor twice on failure.

Modified:
  stable/7/contrib/ipfilter/   (props changed)
  stable/7/contrib/ipfilter/lib/load_http.c

Modified: stable/7/contrib/ipfilter/lib/load_http.c
==============================================================================
--- stable/7/contrib/ipfilter/lib/load_http.c	Mon Jun 15 20:45:51 2009	(r194264)
+++ stable/7/contrib/ipfilter/lib/load_http.c	Mon Jun 15 21:03:25 2009	(r194265)
@@ -14,11 +14,13 @@
 alist_t *
 load_http(char *url)
 {
-	int fd, len, left, port, endhdr, removed;
-	char *s, *t, *u, buffer[1024], *myurl;
+	char *s, *t, *u, buffer[1044], *myurl;
 	alist_t *a, *rtop, *rbot;
 	struct sockaddr_in sin;
 	struct hostent *host;
+	size_t avail;
+	int fd, len, left, port, endhdr, removed;
+	int error;
 
 	/*
 	 * More than this would just be absurd.
@@ -32,7 +34,14 @@ load_http(char *url)
 	rtop = NULL;
 	rbot = NULL;
 
-	sprintf(buffer, "GET %s HTTP/1.0\r\n", url);
+	avail = sizeof(buffer);
+	error = snprintf(buffer, avail, "GET %s HTTP/1.0\r\n", url);
+
+	/*
+	 * error is always less then avail due to the constraint on
+	 * the url length above.
+	 */
+	avail -= error;
 
 	myurl = strdup(url);
 	if (myurl == NULL)
@@ -51,7 +60,11 @@ load_http(char *url)
 	if (u != NULL)
 		s = u + 1;		/* AUTH */
 
-	sprintf(buffer + strlen(buffer), "Host: %s\r\n\r\n", s);
+	error = snprintf(buffer + strlen(buffer), avail, "Host: %s\r\n\r\n", s);
+	if (error >= avail) {
+		fprintf(stderr, "URL is too large: %s\n", url);
+		goto done;
+	}
 
 	u = strchr(s, ':');
 	if (u != NULL) {
@@ -83,16 +96,12 @@ load_http(char *url)
 	if (fd == -1)
 		goto done;
 
-	if (connect(fd, (struct sockaddr *)&sin, sizeof(sin)) == -1) {
-		close(fd);
+	if (connect(fd, (struct sockaddr *)&sin, sizeof(sin)) == -1)
 		goto done;
-	}
 
 	len = strlen(buffer);
-	if (write(fd, buffer, len) != len) {
-		close(fd);
+	if (write(fd, buffer, len) != len)
 		goto done;
-	}
 
 	s = buffer;
 	endhdr = 0;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200906152103.n5FL3Q3Z001871>