open-src/lib/libXv/CVE-2013-1989.patch
author Alan Coopersmith <Alan.Coopersmith@Oracle.COM>
Wed, 15 May 2013 13:44:02 -0700
changeset 1345 d5dacbb8de2b
permissions -rw-r--r--
16673783 problem in X11/LIBRARIES 16674478 problem in X11/LIBRARIES

From 79362c764a6df7e7fbe5247756bdbf60f3a58baf Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <[email protected]>
Date: Sat, 13 Apr 2013 00:28:34 -0700
Subject: [PATCH:libXv 1/5] Use _XEatDataWords to avoid overflow of rep.length
 shifting

rep.length is a CARD32, so rep.length << 2 could overflow in 32-bit builds

Signed-off-by: Alan Coopersmith <[email protected]>
---
 configure.ac |    6 ++++++
 src/Xv.c     |   22 +++++++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5494b5d..6a335db 100644
--- a/configure.ac
+++ b/configure.ac
@@ -43,6 +43,12 @@ XORG_CHECK_MALLOC_ZERO
 # Obtain compiler/linker options for depedencies
 PKG_CHECK_MODULES(XV, x11 xext xextproto videoproto)
 
+# Check for _XEatDataWords function that may be patched into older Xlib release
+SAVE_LIBS="$LIBS"
+LIBS="$XV_LIBS"
+AC_CHECK_FUNCS([_XEatDataWords])
+LIBS="$SAVE_LIBS"
+
 # Allow checking code with lint, sparse, etc.
 XORG_WITH_LINT
 XORG_LINT_LIBRARY([Xv])
diff --git a/src/Xv.c b/src/Xv.c
index b081e8a..5be1d95 100644
--- a/src/Xv.c
+++ b/src/Xv.c
@@ -49,11 +49,27 @@ SOFTWARE.
 **
 */
 
+#ifdef HAVE_CONFIG_H
+# include "config.h"
+#endif
+
 #include <stdio.h>
 #include "Xvlibint.h"
 #include <X11/extensions/Xext.h>
 #include <X11/extensions/extutil.h>
 #include <X11/extensions/XShm.h>
+#include <limits.h>
+
+#ifndef HAVE__XEATDATAWORDS
+static inline void _XEatDataWords(Display *dpy, unsigned long n)
+{
+# ifndef LONG64
+    if (n >= (ULONG_MAX >> 2))
+        _XIOError(dpy);
+# endif
+    _XEatData (dpy, n << 2);
+}
+#endif
 
 static XExtensionInfo _xv_info_data;
 static XExtensionInfo *xv_info = &_xv_info_data;
@@ -853,7 +869,7 @@ XvQueryPortAttributes(Display *dpy, XvPortID port, int *num)
 	      (*num)++;
 	  }
       } else
-	_XEatData(dpy, rep.length << 2);
+	  _XEatDataWords(dpy, rep.length);
   }
 
   UnlockDisplay(dpy);
@@ -923,7 +939,7 @@ XvImageFormatValues * XvListImageFormats (
 	      (*num)++;
 	  }
       } else
-	_XEatData(dpy, rep.length << 2);
+	  _XEatDataWords(dpy, rep.length);
   }
 
   UnlockDisplay(dpy);
@@ -976,7 +992,7 @@ XvImage * XvCreateImage (
   	_XRead(dpy, (char*)(ret->pitches), rep.num_planes << 2);
 	_XRead(dpy, (char*)(ret->offsets), rep.num_planes << 2);
    } else
-	_XEatData(dpy, rep.length << 2);
+       _XEatDataWords(dpy, rep.length);
 
    UnlockDisplay(dpy);
    SyncHandle();
-- 
1.7.9.2

From 6e1b743a276651195be3cd68dff41e38426bf3ab Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <[email protected]>
Date: Sat, 13 Apr 2013 00:03:03 -0700
Subject: [PATCH:libXv 2/5] integer overflow in XvQueryPortAttributes()
 [CVE-2013-1989 1/3]

The num_attributes & text_size members of the reply are both CARD32s
and need to be bounds checked before multiplying & adding them together
to come up with the total size to allocate, to avoid integer overflow
leading to underallocation and writing data from the network past the
end of the allocated buffer.

Reported-by: Ilja Van Sprundel <[email protected]>
Signed-off-by: Alan Coopersmith <[email protected]>
---
 src/Xv.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/Xv.c b/src/Xv.c
index 5be1d95..3cbad35 100644
--- a/src/Xv.c
+++ b/src/Xv.c
@@ -851,9 +851,15 @@ XvQueryPortAttributes(Display *dpy, XvPortID port, int *num)
   }
 
   if(rep.num_attributes) {
-      int size = (rep.num_attributes * sizeof(XvAttribute)) + rep.text_size;
+      unsigned long size;
+      /* limit each part to no more than one half the max size */
+      if ((rep.num_attributes < ((INT_MAX / 2) / sizeof(XvAttribute))) &&
+	  (rep.text_size < (INT_MAX / 2))) {
+	  size = (rep.num_attributes * sizeof(XvAttribute)) + rep.text_size;
+	  ret = Xmalloc(size);
+      }
 
-      if((ret = Xmalloc(size))) {
+      if (ret != NULL) {
 	  char* marker = (char*)(&ret[rep.num_attributes]);
 	  xvAttributeInfo Info;
 	  int i;
-- 
1.7.9.2

From 15ab7dec17d686c38f2c82ac23a17cac5622322a Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <[email protected]>
Date: Sat, 13 Apr 2013 00:16:14 -0700
Subject: [PATCH:libXv 3/5] buffer overflow in XvQueryPortAttributes()
 [CVE-2013-2066]

Each attribute returned in the reply includes the number of bytes
to read for its marker.  We had been always trusting it, and never
validating that it wouldn't cause us to write past the end of the
buffer we allocated based on the reported text_size.

Reported-by: Ilja Van Sprundel <[email protected]>
Signed-off-by: Alan Coopersmith <[email protected]>
---
 src/Xv.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/Xv.c b/src/Xv.c
index 3cbad35..f9813eb 100644
--- a/src/Xv.c
+++ b/src/Xv.c
@@ -864,14 +864,20 @@ XvQueryPortAttributes(Display *dpy, XvPortID port, int *num)
 	  xvAttributeInfo Info;
 	  int i;
 
+	  /* keep track of remaining room for text strings */
+	  size = rep.text_size;
+
 	  for(i = 0; i < rep.num_attributes; i++) {
              _XRead(dpy, (char*)(&Info), sz_xvAttributeInfo);
 	      ret[i].flags = (int)Info.flags;
 	      ret[i].min_value = Info.min;
 	      ret[i].max_value = Info.max;
 	      ret[i].name = marker;
-	      _XRead(dpy, marker, Info.size);
-	      marker += Info.size;
+	      if (Info.size <= size) {
+		  _XRead(dpy, marker, Info.size);
+		  marker += Info.size;
+		  size -= Info.size;
+	      }
 	      (*num)++;
 	  }
       } else
-- 
1.7.9.2

From 59301c1b5095f7dc6359d5b396dbbcdee7038270 Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <[email protected]>
Date: Sat, 13 Apr 2013 00:03:03 -0700
Subject: [PATCH:libXv 4/5] integer overflow in XvListImageFormats()
 [CVE-2013-1989 2/3]

num_formats is a CARD32 and needs to be bounds checked before multiplying
by sizeof(XvImageFormatValues) to come up with the total size to allocate,
to avoid integer overflow leading to underallocation and writing data from
the network past the end of the allocated buffer.

Reported-by: Ilja Van Sprundel <[email protected]>
Signed-off-by: Alan Coopersmith <[email protected]>
---
 src/Xv.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/Xv.c b/src/Xv.c
index f9813eb..0a07d9d 100644
--- a/src/Xv.c
+++ b/src/Xv.c
@@ -918,9 +918,10 @@ XvImageFormatValues * XvListImageFormats (
   }
 
   if(rep.num_formats) {
-      int size = (rep.num_formats * sizeof(XvImageFormatValues));
+      if (rep.num_formats < (INT_MAX / sizeof(XvImageFormatValues)))
+	  ret = Xmalloc(rep.num_formats * sizeof(XvImageFormatValues));
 
-      if((ret = Xmalloc(size))) {
+      if (ret != NULL) {
 	  xvImageFormatInfo Info;
 	  int i;
 
-- 
1.7.9.2

From 50fc4cb18069cb9450a02c13f80223ef23511409 Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <[email protected]>
Date: Sat, 13 Apr 2013 00:03:03 -0700
Subject: [PATCH:libXv 5/5] integer overflow in XvCreateImage() [CVE-2013-1989
 3/3]

num_planes is a CARD32 and needs to be bounds checked before bit shifting
and adding to sizeof(XvImage) to come up with the total size to allocate,
to avoid integer overflow leading to underallocation and writing data from
the network past the end of the allocated buffer.

Reported-by: Ilja Van Sprundel <[email protected]>
Signed-off-by: Alan Coopersmith <[email protected]>
---
 src/Xv.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/Xv.c b/src/Xv.c
index 0a07d9d..f268f8e 100644
--- a/src/Xv.c
+++ b/src/Xv.c
@@ -992,7 +992,10 @@ XvImage * XvCreateImage (
       return NULL;
    }
 
-   if((ret = (XvImage*)Xmalloc(sizeof(XvImage) + (rep.num_planes << 3)))) {
+   if (rep.num_planes < ((INT_MAX >> 3) - sizeof(XvImage)))
+       ret = Xmalloc(sizeof(XvImage) + (rep.num_planes << 3));
+
+   if (ret != NULL) {
 	ret->id = id;
 	ret->width = rep.width;
 	ret->height = rep.height;
-- 
1.7.9.2