Eliminate broken data end calculation in dataLength()
authorPanu Matilainen <pmatilai@redhat.com>
Fri, 23 Mar 2012 12:17:47 +0000 (14:17 +0200)
committerPanu Matilainen <pmatilai@redhat.com>
Fri, 23 Mar 2012 12:36:34 +0000 (14:36 +0200)
- If the caller doesn't know the end pointer, we dont have a whole lot
  of chance to come up with a reasonable one either. Just assume
  the terminating \0's are there when end boundary is not specified:
  when this happens we're dealing with relatively "trusted" data
  anyway, the more critical case of reading in unknown headers does
  always pass end pointers.
- While capping the end pointer to HEADER_DATA_MAX seems like a
  reasonable thing to do (as was done in commit
  f79909d04e43cbfbbcdc588530a8c8033c5e0a7c), it doesn't really help
  (bad data would likely run past bounds anyway), and it's not right
  either: the pointer can be to a stack address, and the stack can be
  near the top of addressable range, and ptr + HEADER_DATA_MAX can
  cause pointer wraparound. Notably that's exactly what happens
  when running 32bit personality process on 64bit system on Linux,
  at least in case of i386 process on x86_64, causing all sorts of
  breakage..

lib/header.c

index d741552..023c6e3 100644 (file)
@@ -301,16 +301,27 @@ unsigned headerSizeof(Header h, int magicp)
     return size;
 }
 
-/* Bounded header string (array) size calculation, return -1 on error */
+/*
+ * Header string (array) size calculation, bounded if end is non-NULL.
+ * Return length (including \0 termination) on success, -1 on error.
+ */
 static inline int strtaglen(const char *str, rpm_count_t c, const char *end)
 {
     const char *start = str;
     const char *s;
 
-    while ((s = memchr(start, '\0', end-start))) {
-       if (--c == 0 || s > end)
-           break;
-       start = s + 1;
+    if (end) {
+       while ((s = memchr(start, '\0', end-start))) {
+           if (--c == 0 || s > end)
+               break;
+           start = s + 1;
+       }
+    } else {
+       while ((s = strchr(start, '\0'))) {
+           if (--c == 0)
+               break;
+           start = s + 1;
+       }
     }
     return (c > 0) ? -1 : (s - str + 1);
 }
@@ -328,8 +339,7 @@ static int dataLength(rpm_tagtype_t type, rpm_constdata_t p, rpm_count_t count,
                         int onDisk, rpm_constdata_t pend)
 {
     const char * s = p;
-    /* Not all callers supply data end, avoid falling over edge of the world */
-    const char * se = pend ? pend : s + HEADER_DATA_MAX;
+    const char * se = pend;
     int length = 0;
 
     switch (type) {