[PATCH][TOOLS] xenstore: Make code more secure

[PATCH][TOOLS] xenstore: Make code more secure

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]


Hi!

Attached patch adds length checks mainly by replacing sprintf with snprintf
in order to make the code more secure in general.
The little header fiddlings are build fixes for OpenBSD.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

[-- Attachment #2: tools_xenstore.diff --]
[-- Type: text/plain, Size: 5525 bytes --]

diff -r 2d761ca771fb tools/xenstore/xenstored_core.c
--- a/tools/xenstore/xenstored_core.c	Thu Oct 04 17:58:16 2007 +0100
+++ b/tools/xenstore/xenstored_core.c	Fri Oct 05 11:51:50 2007 +0200
@@ -685,7 +685,7 @@ static char *perms_to_strings(const void
 	char buffer[MAX_STRLEN(unsigned int) + 1];
 
 	for (*len = 0, i = 0; i < num; i++) {
-		if (!xs_perm_to_string(&perms[i], buffer))
+		if (!xs_perm_to_string(&perms[i], buffer, sizeof(buffer)))
 			return NULL;
 
 		strings = talloc_realloc(ctx, strings, char,
@@ -1659,7 +1659,7 @@ static void write_pidfile(const char *pi
 	if (lockf(fd, F_TLOCK, 0) == -1)
 		exit(0);
 
-	len = sprintf(buf, "%ld\n", (long)getpid());
+	len = snprintf(buf, sizeof(buf), "%ld\n", (long)getpid());
 	if (write(fd, buf, len) != len)
 		barf_perror("Writing pid file %s", pidfile);
 }
diff -r 2d761ca771fb tools/xenstore/xenstored_transaction.c
--- a/tools/xenstore/xenstored_transaction.c	Thu Oct 04 17:58:16 2007 +0100
+++ b/tools/xenstore/xenstored_transaction.c	Fri Oct 05 11:48:21 2007 +0200
@@ -181,7 +181,7 @@ void do_transaction_start(struct connect
 	talloc_set_destructor(trans, destroy_transaction);
 	conn->transaction_started++;
 
-	sprintf(id_str, "%u", trans->id);
+	snprintf(id_str, sizeof(id_str), "%u", trans->id);
 	send_reply(conn, XS_TRANSACTION_START, id_str, strlen(id_str)+1);
 }
 
diff -r 2d761ca771fb tools/xenstore/xs.c
--- a/tools/xenstore/xs.c	Thu Oct 04 17:58:16 2007 +0100
+++ b/tools/xenstore/xs.c	Fri Oct 05 13:42:44 2007 +0200
@@ -20,6 +20,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <sys/uio.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <string.h>
@@ -519,7 +520,7 @@ bool xs_set_permissions(struct xs_handle
 	for (i = 0; i < num_perms; i++) {
 		char buffer[MAX_STRLEN(unsigned int)+1];
 
-		if (!xs_perm_to_string(&perms[i], buffer))
+		if (!xs_perm_to_string(&perms[i], buffer, sizeof(buffer)))
 			goto unwind;
 
 		iov[i+1].iov_base = strdup(buffer);
@@ -687,9 +688,9 @@ bool xs_introduce_domain(struct xs_handl
 	char eventchn_str[MAX_STRLEN(eventchn)];
 	struct iovec iov[3];
 
-	sprintf(domid_str, "%u", domid);
-	sprintf(mfn_str, "%lu", mfn);
-	sprintf(eventchn_str, "%u", eventchn);
+	snprintf(domid_str, sizeof(domid_str), "%u", domid);
+	snprintf(mfn_str, sizeof(mfn_str), "%lu", mfn);
+	snprintf(eventchn_str, sizeof(eventchn_str), "%u", eventchn);
 
 	iov[0].iov_base = domid_str;
 	iov[0].iov_len = strlen(domid_str) + 1;
@@ -708,7 +709,7 @@ static void * single_with_domid(struct x
 {
 	char domid_str[MAX_STRLEN(domid)];
 
-	sprintf(domid_str, "%u", domid);
+	snprintf(domid_str, sizeof(domid_str), "%u", domid);
 
 	return xs_single(h, XBT_NULL, type, domid_str, NULL);
 }
@@ -728,7 +729,7 @@ char *xs_get_domain_path(struct xs_handl
 {
 	char domid_str[MAX_STRLEN(domid)];
 
-	sprintf(domid_str, "%u", domid);
+	snprintf(domid_str, sizeof(domid_str), "%u", domid);
 
 	return xs_single(h, XBT_NULL, XS_GET_DOMAIN_PATH, domid_str, NULL);
 }
diff -r 2d761ca771fb tools/xenstore/xs_lib.c
--- a/tools/xenstore/xs_lib.c	Thu Oct 04 17:58:16 2007 +0100
+++ b/tools/xenstore/xs_lib.c	Fri Oct 05 11:51:33 2007 +0200
@@ -17,12 +17,12 @@
     Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 */
 
-#include "xs_lib.h"
 #include <unistd.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
 #include <errno.h>
+#include "xs_lib.h"
 
 /* Common routines for the Xen store daemon and client library. */
 
@@ -53,7 +53,7 @@ const char *xs_daemon_tdb(void)
 const char *xs_daemon_tdb(void)
 {
 	static char buf[PATH_MAX];
-	sprintf(buf, "%s/tdb", xs_daemon_rootdir());
+	snprintf(buf, sizeof(buf), "%s/tdb", xs_daemon_rootdir());
 	return buf;
 }
 
@@ -143,7 +143,8 @@ bool xs_strings_to_perms(struct xs_permi
 }
 
 /* Convert permissions to a string (up to len MAX_STRLEN(unsigned int)+1). */
-bool xs_perm_to_string(const struct xs_permissions *perm, char *buffer)
+bool xs_perm_to_string(const struct xs_permissions *perm,
+                       char *buffer, size_t buf_len)
 {
 	switch (perm->perms) {
 	case XS_PERM_WRITE:
@@ -162,7 +163,7 @@ bool xs_perm_to_string(const struct xs_p
 		errno = EINVAL;
 		return false;
 	}
-	sprintf(buffer+1, "%i", (int)perm->id);
+	snprintf(buffer+1, buf_len-1, "%i", (int)perm->id);
 	return true;
 }
 
diff -r 2d761ca771fb tools/xenstore/xs_lib.h
--- a/tools/xenstore/xs_lib.h	Thu Oct 04 17:58:16 2007 +0100
+++ b/tools/xenstore/xs_lib.h	Fri Oct 05 11:50:23 2007 +0200
@@ -61,7 +61,8 @@ bool xs_strings_to_perms(struct xs_permi
 			 const char *strings);
 
 /* Convert permissions to a string (up to len MAX_STRLEN(unsigned int)+1). */
-bool xs_perm_to_string(const struct xs_permissions *perm, char *buffer);
+bool xs_perm_to_string(const struct xs_permissions *perm,
+                       char *buffer, size_t buf_len);
 
 /* Given a string and a length, count how many strings (nul terms). */
 unsigned int xs_count_strings(const char *strings, unsigned int len);
diff -r 2d761ca771fb tools/xenstore/xsls.c
--- a/tools/xenstore/xsls.c	Thu Oct 04 17:58:16 2007 +0100
+++ b/tools/xenstore/xsls.c	Fri Oct 05 11:56:10 2007 +0200
@@ -87,7 +87,7 @@ void print_dir(struct xs_handle *h, char
                 for (i = 0; i < nperms; i++) {
                     if (i)
                         putchar(',');
-                    xs_perm_to_string(perms+i, buf);
+                    xs_perm_to_string(perms+i, buf, sizeof(buf));
                     fputs(buf, stdout);
                 }
                 putchar(')');

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel