Fix segfault and out-of-bounds read in Binary fields

If a Binary field contains one or more spaces before a comma, the
code produced a segmentation fault, as it accidentally set a pointer
to 0 instead of the value of the pointer.

If the comma is at the beginning of the field, the code would
create a binStartNext that points one element before the start
of the string, which is undefined behavior.

We also need to check that we do not exit the string during the
replacement of spaces before commas: A string of the form " ,"
would normally exit the boundary of the Buffer:

	binStartNext = offset 1 ','
	binEnd = offset 0	' '
	isspace_ascii(*binEnd) = true => --binEnd
				      => binEnd = - 1

We get rid of the problem by only allowing spaces to be eliminated
if they are not the first character of the buffer:

        binStartNext = offset 1 ','
        binEnd = offset 0       ' '
        binEnd > buffer = false, isspace_ascii(*binEnd) = true
		 => exit loop
                => binEnd remains 0

diff --git a/apt-pkg/deb/debsrcrecords.cc b/apt-pkg/deb/debsrcrecords.cc
index 5454d79c3de9164f0a7d12f842fa69840f7581c6..d296161d6aad203c1dff8ce18fe826f3851b6088 100644 (file)
--- a/apt-pkg/deb/debsrcrecords.cc
+++ b/apt-pkg/deb/debsrcrecords.cc
@@ -73,9 +73,12 @@ const char **debSrcRecordParser::Binaries()
    char* bin = Buffer;
    do {
       char* binStartNext = strchrnul(bin, ',');
    char* bin = Buffer;
    do {
       char* binStartNext = strchrnul(bin, ',');

-      char* binEnd = binStartNext - 1;
-      for (; isspace_ascii(*binEnd) != 0; --binEnd)
-        binEnd = 0;
+      // Found a comma, clean up any space before it
+      if (binStartNext > Buffer) {
+        char* binEnd = binStartNext - 1;
+        for (; binEnd > Buffer && isspace_ascii(*binEnd) != 0; --binEnd)
+           *binEnd = 0;
+      }

       StaticBinList.push_back(bin);
       if (*binStartNext != ',')
         break;
       StaticBinList.push_back(bin);
       if (*binStartNext != ',')
         break;

diff --git a/test/integration/test-srcrecord b/test/integration/test-srcrecord
new file mode 100644 (file)
index 0000000..34de2be
--- /dev/null
+++ b/test/integration/test-srcrecord
@@ -0,0 +1,35 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+
+setupenvironment
+configarchitecture 'native'
+
+cat > aptarchive/Sources <<EOF
+Package: space-before-comma
+Binary: space-before-comma1 , space-before-comma2
+Version: 1.0
+Maintainer: Joe Sixpack <joe@example.org>
+Architecture: all
+
+Package: broken-field
+Binary:, broken-field2
+Version: 1.0
+Maintainer: Joe Sixpack <joe@example.org>
+Architecture: all
+
+Package: broken-field-b
+Binary: , broken-field-b2
+Version: 1.0
+Maintainer: Joe Sixpack <joe@example.org>
+Architecture: all
+EOF
+
+setupaptarchive --no-update
+
+testsuccess aptget update
+testsuccess aptcache showsrc space-before-comma1
+testsuccess aptcache showsrc broken-field2
+testsuccess aptcache showsrc broken-field-b2