summaryrefslogtreecommitdiff
authorSimon McVittie <simon.mcvittie@collabora.co.uk>2013-06-10 17:06:47 (GMT)
committer Simon McVittie <simon.mcvittie@collabora.co.uk>2013-06-12 12:55:53 (GMT)
commit954d75b2b64e4799f360d2a6bf9cff6d9fee37e7 (patch) (side-by-side diff)
treeb2ce9ace5bc08528c4c1df851147f865159ac056
parent355b470da78e25cb451eab0c49f30437b2c5ccb9 (diff)
downloaddbus-954d75b2b64e4799f360d2a6bf9cff6d9fee37e7.zip
dbus-954d75b2b64e4799f360d2a6bf9cff6d9fee37e7.tar.gz
CVE-2013-2168: _dbus_printf_string_upper_bound: copy the va_list for each use
Using a va_list more than once is non-portable: it happens to work under the ABI of (for instance) x86 Linux, but not x86-64 Linux. This led to _dbus_printf_string_upper_bound() crashing if it should have returned exactly 1024 bytes. Many system services can be induced to process a caller-controlled string in ways that end up using _dbus_printf_string_upper_bound(), so this is a denial of service. Reviewed-by: Thiago Macieira <thiago@kde.org>
Diffstat (more/less context) (ignore whitespace changes)
-rw-r--r--dbus/dbus-sysdeps-unix.c16
-rw-r--r--dbus/dbus-sysdeps-win.c9
2 files changed, 20 insertions, 5 deletions
diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
index fc67799..e31c735 100644
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -3121,8 +3121,11 @@ _dbus_printf_string_upper_bound (const char *format,
char static_buf[1024];
int bufsize = sizeof (static_buf);
int len;
+ va_list args_copy;
- len = vsnprintf (static_buf, bufsize, format, args);
+ DBUS_VA_COPY (args_copy, args);
+ len = vsnprintf (static_buf, bufsize, format, args_copy);
+ va_end (args_copy);
/* If vsnprintf() returned non-negative, then either the string fits in
* static_buf, or this OS has the POSIX and C99 behaviour where vsnprintf
@@ -3138,8 +3141,12 @@ _dbus_printf_string_upper_bound (const char *format,
* or the real length could be coincidentally the same. Which is it?
* If vsnprintf returns the truncated length, we'll go to the slow
* path. */
- if (vsnprintf (static_buf, 1, format, args) == 1)
+ DBUS_VA_COPY (args_copy, args);
+
+ if (vsnprintf (static_buf, 1, format, args_copy) == 1)
len = -1;
+
+ va_end (args_copy);
}
/* If vsnprintf() returned negative, we have to do more work.
@@ -3155,7 +3162,10 @@ _dbus_printf_string_upper_bound (const char *format,
if (buf == NULL)
return -1;
- len = vsnprintf (buf, bufsize, format, args);
+ DBUS_VA_COPY (args_copy, args);
+ len = vsnprintf (buf, bufsize, format, args_copy);
+ va_end (args_copy);
+
dbus_free (buf);
/* If the reported length is exactly the buffer size, round up to the
diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c
index bc4951b..c42316f 100644
--- a/dbus/dbus-sysdeps-win.c
+++ b/dbus/dbus-sysdeps-win.c
@@ -538,9 +538,12 @@ int _dbus_printf_string_upper_bound (const char *format,
char buf[1024];
int bufsize;
int len;
+ va_list args_copy;
bufsize = sizeof (buf);
- len = _vsnprintf (buf, bufsize - 1, format, args);
+ DBUS_VA_COPY (args_copy, args);
+ len = _vsnprintf (buf, bufsize - 1, format, args_copy);
+ va_end (args_copy);
while (len == -1) /* try again */
{
@@ -553,7 +556,9 @@ int _dbus_printf_string_upper_bound (const char *format,
if (p == NULL)
return -1;
- len = _vsnprintf (p, bufsize - 1, format, args);
+ DBUS_VA_COPY (args_copy, args);
+ len = _vsnprintf (p, bufsize - 1, format, args_copy);
+ va_end (args_copy);
free (p);
}