t51 and t52 report "buffer overflow detected" when built with -D_FORTIFY_SOURCE enabled

Bug #1744933 reported by James McCoy
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
libtickit
Fix Released
Undecided
Unassigned

Bug Description

I ran into this while working on the packaging for Debian.

From a fresh checkout:

$ env CFLAGS="-g -O2 -D_FORTIFY_SOURCE=2" make test
...
t/51tickit-timer.t ........ *** buffer overflow detected ***: /home/jamessan/src/leonerd.org.uk/libtickit/t/.libs/lt-51tickit-timer.t terminated
t/51tickit-timer.t ........ No subtests run
t/52tickit-later.t ........ *** buffer overflow detected ***: /home/jamessan/src/leonerd.org.uk/libtickit/t/.libs/lt-52tickit-later.t terminated
t/52tickit-later.t ........ No subtests run

$ libtool e gdb --args ./t/51tickit-timer.t
...
(gdb) run
Starting program: /home/jamessan/src/leonerd.org.uk/libtickit/t/.libs/lt-51tickit-timer.t
*** buffer overflow detected ***: /home/jamessan/src/leonerd.org.uk/libtickit/t/.libs/lt-51tickit-timer.t terminated

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ffff783ccf7 in __GI_abort () at abort.c:90
#2 0x00007ffff787df87 in __libc_message (action=<optimized out>,
    fmt=fmt@entry=0x7ffff79839ee "*** %s ***: %s terminated\n")
    at ../sysdeps/posix/libc_fatal.c:181
#3 0x00007ffff790d7de in __GI___fortify_fail_abort (need_backtrace=need_backtrace@entry=true,
    msg=msg@entry=0x7ffff798396b "buffer overflow detected") at fortify_fail.c:33
#4 0x00007ffff790d811 in __GI___fortify_fail (
    msg=msg@entry=0x7ffff798396b "buffer overflow detected") at fortify_fail.c:44
#5 0x00007ffff790b6d0 in __GI___chk_fail () at chk_fail.c:28
#6 0x00007ffff790d71a in __fdelt_chk (d=d@entry=-1) at fdelt_chk.c:25
#7 0x00007ffff7bcad60 in tickit_term_input_wait_msec (tt=0x55555579aef0, msec=9)
    at src/term.c:683
#8 0x00007ffff7bce2b9 in tickit_run (t=0x55555579b000) at src/tickit.c:195
#9 0x0000555555555044 in main (argc=1, argv=0x7fffffffe7e8) at t/51tickit-timer.c:47
(gdb) frame 7
#7 0x00007ffff7bcad60 in tickit_term_input_wait_msec (tt=0x55555579aef0, msec=9)
    at src/term.c:683
683 FD_SET(fd, &readfds);
(gdb) l
678 timeout.tv_usec = (msec % 1000) * 1000;
679 }
680
681 fd_set readfds;
682 int fd = termkey_get_fd(tk);
683 FD_SET(fd, &readfds);
684 int ret = select(fd + 1, &readfds, NULL, NULL, msec > -1 ? &timeout : NULL);
685
686 if(ret == 0)
687 timedout(tt);
(gdb) p fd
$1 = -1

Revision history for this message
James McCoy (jamessan) wrote :

As suggested in the gdb logs, termkey_get_fd() is returning -1, which results in invalid indexing of readfds.

Steve Langasek supplied the attached patch to be more defensive about the value returned from termkey_get_key(). This avoids the invalid indexing when -1 is returned, but this part of the code should be reviewed to see if it's expected to get a -1 value.

Revision history for this message
James McCoy (jamessan) wrote :

Additionally, here's another patch from Steve which ensures the tests are built with the supplied CFLAGS/LDFLAGS.

Revision history for this message
Paul "LeoNerd" Evans (leonerd) wrote :

Thanks. Now applied (manually, with some splitting)

Changed in libtickit:
status: New → Fix Committed
Changed in libtickit:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.