Package: grok
Version: 1.20110708.1-4.1
Severity: important
Tags: patch
User: ubuntu-de...@lists.ubuntu.com
Usertags: origin-ubuntu zesty ubuntu-patch

Hi Stig,

While debugging a build failure of syslog-ng-incubator on s390x in Ubuntu,
I've identified a bug in the grok code related to pointer aliasing.  The
regexp_len argument to grok_pattern_find() has been defined as a size_t*,
but this pointer is then passed through to a tokyocabinet API that only
takes an int*, which leaves half of the size_t variable uninitialized on
64-bit architectures.  And on big-endian architectures, crucially, it's the
lower half of the variable that's uninitialized, eventually leading to a
crash.

The attached patch fixes this by explicitly declaring the API to be based on
an int* instead of a size_t*, and fixing the internal callers of
grok_pattern_find().  Note that this is an API change (changing the
prototype of a public function), but is not an ABI change; I'm only changing
the public declaration to match the actual behavior.  Therefore, if there
are other external users of this function which are assuming they can pass
in a size_t* to uninitialized memory and use it afterwards, those callers
will still see buggy behavior.  So you (or upstream) may wish instead to
keep the prototype as-is, and fix up the pointer aliasing within the
function.

Thanks for considering the patch!
-- 
Steve Langasek                   Give me a lever long enough and a Free OS
Debian Developer                   to set it on, and I can move the world.
Ubuntu Developer                                    http://www.debian.org/
slanga...@ubuntu.com                                     vor...@debian.org
diff -Nru grok-1.20110708.1/debian/patches/fix_wrong_pointer_alias.patch grok-1.20110708.1/debian/patches/fix_wrong_pointer_alias.patch
--- grok-1.20110708.1/debian/patches/fix_wrong_pointer_alias.patch	1969-12-31 16:00:00.000000000 -0800
+++ grok-1.20110708.1/debian/patches/fix_wrong_pointer_alias.patch	2016-10-21 14:05:10.000000000 -0700
@@ -0,0 +1,64 @@
+Description: fix wrong pointer alias
+ size_t * != int *, and casting one to the other without initialization is
+ a good way to get garbage data into your program, leading to crashes such
+ as the one in
+ https://launchpad.net/ubuntu/+source/syslog-ng-incubator/0.5.0-1build1/+build/11039099
+Author: Steve Langasek <steve.langa...@ubuntu.com>
+Forwarded: no
+Last-Update: 2016-10-21
+
+Index: grok-1.20110708.1/grok_pattern.c
+===================================================================
+--- grok-1.20110708.1.orig/grok_pattern.c
++++ grok-1.20110708.1/grok_pattern.c
+@@ -33,9 +33,9 @@
+ }
+ 
+ int grok_pattern_find(const grok_t *grok, const char *name, size_t name_len,
+-                      const char **regexp, size_t *regexp_len) {
++                      const char **regexp, int *regexp_len) {
+   TCTREE *patterns = grok->patterns;
+-  *regexp = tctreeget(patterns, name, name_len, (int*) regexp_len);
++  *regexp = tctreeget(patterns, name, name_len, regexp_len);
+ 
+   grok_log(grok, LOG_PATTERNS, "Searching for pattern '%s' (%s): %.*s",
+            name, *regexp == NULL ? "not found" : "found", *regexp_len, *regexp);
+Index: grok-1.20110708.1/grok_pattern.h
+===================================================================
+--- grok-1.20110708.1.orig/grok_pattern.h
++++ grok-1.20110708.1/grok_pattern.h
+@@ -9,7 +9,7 @@
+ int grok_pattern_add(const grok_t *grok, const char *name, size_t name_len,
+                       const char *regexp, size_t regexp_len);
+ int grok_pattern_find(const grok_t *grok, const char *name, size_t name_len,
+-                      const char **regexp, size_t *regexp_len);
++                      const char **regexp, int *regexp_len);
+ int grok_patterns_import_from_file(const grok_t *grok, const char *filename);
+ int grok_patterns_import_from_string(const grok_t *grok, const char *buffer);
+ 
+Index: grok-1.20110708.1/test/grok_pattern.test.c
+===================================================================
+--- grok-1.20110708.1.orig/test/grok_pattern.test.c
++++ grok-1.20110708.1/test/grok_pattern.test.c
+@@ -4,7 +4,7 @@
+ void test_grok_pattern_add_and_find_work(void) {
+   INIT;
+   const char *regexp = NULL;
+-  size_t len = 0;
++  int len = 0;
+ 
+   grok_pattern_add(&grok, "WORD", 5, "\\w+", 3);
+   grok_pattern_add(&grok, "TEST", 5, "TEST", 4);
+Index: grok-1.20110708.1/grokre.c
+===================================================================
+--- grok-1.20110708.1.orig/grokre.c
++++ grok-1.20110708.1/grokre.c
+@@ -183,7 +183,7 @@
+     int start, end, matchlen;
+     const char *pattern_regex;
+     int patname_len;
+-    size_t regexp_len;
++    int regexp_len;
+     int pattern_regex_needs_free = 0;
+ 
+     grok_log(grok, LOG_REGEXPAND, "% 20s: %.*s", "start of loop",
diff -Nru grok-1.20110708.1/debian/patches/series grok-1.20110708.1/debian/patches/series
--- grok-1.20110708.1/debian/patches/series	2015-01-26 01:57:27.000000000 -0800
+++ grok-1.20110708.1/debian/patches/series	2016-10-21 13:48:31.000000000 -0700
@@ -3,3 +3,4 @@
 0002-Support-GNU-Hurd-add-necessary-linker-flag.patch
 pcre-group-name.patch
 ld-as-needed.diff
+fix_wrong_pointer_alias.patch

Reply via email to