Package: libtickit
Version: 0.2-2
Severity: important
Tags: patch
User: ubuntu-de...@lists.ubuntu.com
Usertags: origin-ubuntu bionic ubuntu-patch

Hi James,

You filed https://bugs.launchpad.net/libtickit/+bug/1744933 about tests
reporting a buffer overflow in libtickit.  It seems you worked around this
by disabling the hardening flags - or at least attempting to, which was
ineffective in Ubuntu because -D_FORTIFY_SOURCE=2 is a compiler built-in in
Ubuntu; which is how I noticed this, because the package still failed to
build in Ubuntu.

I dug into the build failure, and this looks like a genuine out-of-bounds
write in the use of FD_SET() in src/term.c (i.e. the source, not the
tests).  An attacker can likely only cause the fd to be set to -1 rather
than to an arbitrary value, so it's not necessarily exploitable, but the
code does currently allow for scribbling into memory where it shouldn't, so
that's not good.

I've pushed the attached patch to Ubuntu, which fixes the build failure and
also drops the override of -fortify from the hardening flags.

Hope that helps,
-- 
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 libtickit-0.2/debian/patches/cflags-for-tests.patch 
libtickit-0.2/debian/patches/cflags-for-tests.patch
--- libtickit-0.2/debian/patches/cflags-for-tests.patch 1969-12-31 
16:00:00.000000000 -0800
+++ libtickit-0.2/debian/patches/cflags-for-tests.patch 2018-04-08 
22:17:54.000000000 -0700
@@ -0,0 +1,20 @@
+Description: include configured compiler flags when building test cases
+ The test suite fails on Ubuntu because the default buildflags there cause
+ a _FORTIFY_SOURCE failure.  Pass $CFLAGS and $LDFLAGS when building the
+ test cases, to correct this flag mismatch.
+Author: Steve Langasek <steve.langa...@ubuntu.com>
+Last-Modified: 2018-04-08 
+
+Index: libtickit-0.2/Makefile
+===================================================================
+--- libtickit-0.2.orig/Makefile
++++ libtickit-0.2/Makefile
+@@ -73,7 +73,7 @@
+       perl $^ > $@
+ 
+ t/%.t: t/%.c $(LIBRARY) t/taplib.lo t/mockterm.lo t/taplib-tickit.lo
+-      $(LIBTOOL) --mode=link --tag=CC gcc -o $@ -Iinclude -std=c99 -ggdb $^
++      $(LIBTOOL) --mode=link --tag=CC gcc $(CFLAGS) $(LDFLAGS) -o $@ 
-Iinclude -std=c99 -ggdb $^
+ 
+ t/%.lo: t/%.c
+       $(LIBTOOL) --mode=compile --tag=CC gcc $(CFLAGS) -o $@ -c $^
diff -Nru libtickit-0.2/debian/patches/fix-buffer-overflow.patch 
libtickit-0.2/debian/patches/fix-buffer-overflow.patch
--- libtickit-0.2/debian/patches/fix-buffer-overflow.patch      1969-12-31 
16:00:00.000000000 -0800
+++ libtickit-0.2/debian/patches/fix-buffer-overflow.patch      2018-04-09 
00:07:35.000000000 -0700
@@ -0,0 +1,21 @@
+Description: fix out-of-bounds write with fd_set[-1]
+ If termkey_get_fd() fails, we shouldn't try to call select for an fd of -1.
+ Just return instead of crashing on undefined behavior.
+Author: Steve Langasek <steve.langa...@ubuntu.com>
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1744933
+Last-Modified: 2018-04-09
+
+Index: libtickit-0.2/src/term.c
+===================================================================
+--- libtickit-0.2.orig/src/term.c
++++ libtickit-0.2/src/term.c
+@@ -679,7 +679,9 @@
+   }
+ 
+   fd_set readfds;
++  FD_ZERO(&readfds);
+   int fd = termkey_get_fd(tk);
++  if (fd < 0 || fd >= FD_SETSIZE) return;
+   FD_SET(fd, &readfds);
+   int ret = select(fd + 1, &readfds, NULL, NULL, msec > -1 ? &timeout : NULL);
+ 
diff -Nru libtickit-0.2/debian/patches/series 
libtickit-0.2/debian/patches/series
--- libtickit-0.2/debian/patches/series 1969-12-31 16:00:00.000000000 -0800
+++ libtickit-0.2/debian/patches/series 2018-04-08 23:56:27.000000000 -0700
@@ -0,0 +1,2 @@
+cflags-for-tests.patch
+fix-buffer-overflow.patch
diff -Nru libtickit-0.2/debian/rules libtickit-0.2/debian/rules
--- libtickit-0.2/debian/rules  2018-02-05 05:06:30.000000000 -0800
+++ libtickit-0.2/debian/rules  2018-04-09 00:07:43.000000000 -0700
@@ -1,7 +1,7 @@
 #!/usr/bin/make -f
 # -*- makefile -*-
 
-export DEB_BUILD_MAINT_OPTIONS=hardening=+all,-fortify
+export DEB_BUILD_MAINT_OPTIONS=hardening=+all
 
 include /usr/share/dpkg/default.mk
 

Reply via email to