summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJulian Andres Klode <jak@debian.org>2016-08-31 11:36:44 +0200
committerJulian Andres Klode <jak@debian.org>2016-08-31 12:12:56 +0200
commitce6cd75dc367b92f65e4fb539dd166d0f3361f8c (patch)
tree7e0e708bf9862a8a0020d50dfb79a3dd8ecbf894
parent317bb39f3cd6626c74f25d7bdf2907f1b235f553 (diff)
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
-rw-r--r--apt-pkg/deb/debsrcrecords.cc9
-rw-r--r--test/integration/test-srcrecord35
2 files changed, 41 insertions, 3 deletions
diff --git a/apt-pkg/deb/debsrcrecords.cc b/apt-pkg/deb/debsrcrecords.cc
index 5454d79c3..d296161d6 100644
--- 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* 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;
diff --git a/test/integration/test-srcrecord b/test/integration/test-srcrecord
new file mode 100644
index 000000000..34de2be72
--- /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