From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Christoph Egger" Subject: [PATCH][TOOLS] xenstore: Make code more secure Date: Fri, 5 Oct 2007 14:09:25 +0200 Message-ID: <200710051409.25642.Christoph.Egger@amd.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="Boundary-00=_1liBHd3wQoS1Gto" Return-path: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org --Boundary-00=_1liBHd3wQoS1Gto Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline 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 =2D-=20 AMD Saxony, Dresden, Germany Operating System Research Center Legal Information: AMD Saxony Limited Liability Company & Co. KG Sitz (Gesch=E4ftsanschrift): Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland Registergericht Dresden: HRA 4896 vertretungsberechtigter Komplement=E4r: AMD Saxony LLC (Sitz Wilmington, Delaware, USA) Gesch=E4ftsf=FChrer der AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy --Boundary-00=_1liBHd3wQoS1Gto Content-Type: text/plain; charset=us-ascii; name=tools_xenstore.diff Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=tools_xenstore.diff 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 #include #include +#include #include #include #include @@ -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 #include #include #include #include +#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(')'); --Boundary-00=_1liBHd3wQoS1Gto Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --Boundary-00=_1liBHd3wQoS1Gto--