[Nagiosplug-devel] [ nagiosplug-Bugs-1999319 ] check_ntp_peer buffer overflow

[SourceForge.net]

Bugs item #1999319, was opened at 2008-06-21 02:04
Message generated for change (Settings changed) made by dermoth
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=397597&aid=1999319&group_id=29880

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: General plugin execution
Group: Release (specify)
>Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Christian Heim (g_phreak)
Assigned to: Thomas Guyot (dermoth)
Summary: check_ntp_peer buffer overflow

Initial Comment:
check_ntp_peer v1991 (nagios-plugins 1.4.12)

commandline: /usr/lib/nagios/plugins/check_ntp_peer -H ptbtime1.ptb.de -w 120 -c 240

OS: SLES10 SP2 (i586)
GCC: gcc-4.1.2_20070115-0.21
GLIBC: glibc-2.4-31.54

gdb backtrace:
(gdb) file /usr/lib/nagios/plugins/check_ntp_peer
Reading symbols from /usr/lib/nagios/plugins/check_ntp_peer...Reading symbols from /usr/lib/debug/usr/lib/nagios/plugins/check_ntp_peer.debug...done.
Using host libthread_db library "/lib/libthread_db.so.1".
done.
(gdb) set args -H ptbtime1.ptb.de -w 120 -c 240
(gdb) run
Starting program: /usr/lib/nagios/plugins/check_ntp_peer -H ptbtime1.ptb.de -w 120 -c 240
*** buffer overflow detected ***: /usr/lib/nagios/plugins/check_ntp_peer terminated
======= Backtrace: =========
/lib/libc.so.6(__chk_fail+0x41)[0xb7e89071]
/lib/libc.so.6(__read_chk+0x50)[0xb7e89510]
/usr/lib/nagios/plugins/check_ntp_peer[0x8049e9b]
/usr/lib/nagios/plugins/check_ntp_peer[0x804a95e]
/lib/libc.so.6(__libc_start_main+0xdc)[0xb7dcc8ac]
/usr/lib/nagios/plugins/check_ntp_peer[0x8048e71]
======= Memory map: ========
08048000-0804f000 r-xp 00000000 08:02 346900     /usr/lib/nagios/plugins/check_ntp_peer
0804f000-08050000 rw-p 00006000 08:02 346900     /usr/lib/nagios/plugins/check_ntp_peer
08050000-08071000 rw-p 08050000 00:00 0          [heap]
b7d76000-b7d80000 r-xp 00000000 08:02 180638     /lib/libgcc_s.so.1
b7d80000-b7d81000 rw-p 00009000 08:02 180638     /lib/libgcc_s.so.1
b7d81000-b7db6000 r--s 00000000 08:02 383495     /var/run/nscd/dbMurBjs (deleted)
b7db6000-b7db7000 rw-p b7db6000 00:00 0
b7db7000-b7edc000 r-xp 00000000 08:02 180596     /lib/libc-2.4.so
b7edc000-b7ede000 r--p 00124000 08:02 180596     /lib/libc-2.4.so
b7ede000-b7ee0000 rw-p 00126000 08:02 180596     /lib/libc-2.4.so
b7ee0000-b7ee4000 rw-p b7ee0000 00:00 0
b7ee4000-b7ee6000 r-xp 00000000 08:02 180602     /lib/libdl-2.4.so
b7ee6000-b7ee8000 rw-p 00001000 08:02 180602     /lib/libdl-2.4.so
b7ee8000-b7f0b000 r-xp 00000000 08:02 180604     /lib/libm-2.4.so
b7f0b000-b7f0d000 rw-p 00022000 08:02 180604     /lib/libm-2.4.so
b7f0d000-b7f1c000 r-xp 00000000 08:02 180624     /lib/libresolv-2.4.so
b7f1c000-b7f1e000 rw-p 0000e000 08:02 180624     /lib/libresolv-2.4.so
b7f1e000-b7f20000 rw-p b7f1e000 00:00 0
b7f20000-b7f31000 r-xp 00000000 08:02 180607     /lib/libnsl-2.4.so
b7f31000-b7f33000 rw-p 00010000 08:02 180607     /lib/libnsl-2.4.so
b7f33000-b7f35000 rw-p b7f33000 00:00 0
b7f3e000-b7f3f000 rw-p b7f3e000 00:00 0
b7f3f000-b7f59000 r-xp 00000000 08:02 180589     /lib/ld-2.4.so
b7f59000-b7f5b000 rw-p 0001a000 08:02 180589     /lib/ld-2.4.so
bf962000-bf978000 rw-p bf962000 00:00 0          [stack]
ffffe000-fffff000 ---p 00000000 00:00 0          [vdso]

Program received signal SIGABRT, Aborted.
0xffffe410 in __kernel_vsyscall ()
(gdb) bt
#0  0xffffe410 in __kernel_vsyscall ()
#1  0xb7ddf8d0 in raise () from /lib/libc.so.6
#2  0xb7de0ff3 in abort () from /lib/libc.so.6
#3  0xb7e14f9b in __libc_message () from /lib/libc.so.6
#4  0xb7e89071 in __chk_fail () from /lib/libc.so.6
#5  0xb7e89510 in __read_chk () from /lib/libc.so.6
#6  0x08049e9b in ntp_request (host=0x8050130 "ptbtime1.ptb.de", offset=0xbf9745a0, offset_result=0xbf9745b4, jitter=0xbf974598, stratum=0xbf9745b0) at /usr/include/bits/unistd.h:34
#7  0x0804a95e in main (argc=7, argv=0xbf974664) at check_ntp_peer.c:572
#8  0xb7dcc8ac in __libc_start_main () from /lib/libc.so.6
#9  0x08048e71 in _start ()

Thanks a lot.

----------------------------------------------------------------------

Comment By: Thomas Guyot (dermoth)
Date: 2008-11-18 14:46

Message:
Awww. IIRC I hands-calculated the SIZEOF_NTPCM formula and couldn't see
anything wrong. I probably didn't realize the condition was the problem
without even looking at it...

 131 /* NTP control message header is 12 bytes, plus any data in the data
 132  * field, plus null padding to the nearest 32-bit boundary per rfc.
 133  */

Indeed, when the modulo is zero, 4-(ntohs(m.count)%4) returns 4, which
adds 4 more bytes of "rounding" to the result ("m.count" is always true if
count > 0)!!

The correct define should probably be:

#define SIZEOF_NTPCM(m)
(12+ntohs(m.count)+((ntohs(m.count)%4)?4-(ntohs(m.count)%4):0))

In other words, if there's no modulo use "0" instead of the formula.

I'll get that fixed soon. I should probably learn how to use a debugger ;)

Thanks Jamie for pointing it out, and Thierry for  forwarding the post!

----------------------------------------------------------------------

Comment By: Thierry Carrez (tcarrez)
Date: 2008-11-18 11:49

Message:
A similar bug was filed on Ubuntu bugtracker:
https://bugs.launchpad.net/ubuntu/+source/nagios-plugins/+bug/291265
Here is the analysis of Jamie Strandboge about it:

-------------------
I looked at this a bit, and the math seems to be wrong in this line:
#define SIZEOF_NTPCM(m)
(12+ntohs(m.count)+((m.count)?4-(ntohs(m.count)%4):0))

In ntp_request we have (where MAX_CM_SIZE is defined as 468):
req.count=htons(MAX_CM_SIZE);

Which makes req.count = 54273. Later, we have:
if(read(conn, &req, SIZEOF_NTPCM(req)) == -1)

So the nbytes for read() ends up being:
(12 + 468 + (4 - 0)) = 484

However, a sizeof(req) reveals that it is 480 bytes (this can also be seen
by looking at the ntp_control_message struct (1+1+2+2+2+2+2+468)). This is
not security relevant, because the 4 bytes that are overwritten end up
being the 'conn' file descriptor (as seen from gdb), which triggers read()
to:

read(3, 0xbffff850, 484) = ? ERESTARTSYS (To be restarted)
--- SIGALRM (Alarm clock) @ 0 (0) ---

resulting in check_ntp_peer to error out with:
CRITICAL - Socket timeout after 10 seconds

This is a bug whether or not _FORTIFY_SOURCE is used, because read() may
SIGALRM. You'll also notice that check_ntp.c suffers from the same problem
(the code in question is identical), as seen with:
$ /usr/lib/nagios/plugins/check_ntp -H foo -j 1
------------------

----------------------------------------------------------------------

Comment By: SourceForge Robot (sf-robot)
Date: 2008-09-12 22:20

Message:
This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

----------------------------------------------------------------------

Comment By: Thomas Guyot (dermoth)
Date: 2008-08-29 00:12

Message:
Logged In: YES 
user_id=375623
Originator: NO

Although I did reproduce the bug once I couldn't a few weeks later
(library/compiler updates??). I couldn't either on a SLES test system
kindly provided for testing.

>From the backtrace, we're failing __read_chk (_FORTIFY_SOURCE for the
read() call) in ntp_request. There's a single read() in it and I
re-calculated everything by hand: the read data should no exceed allocated
memory for req.

This seems like a bug in the _FORTIFY_SOURCE checks. Marking as pending as
I'm unable to move forward.

----------------------------------------------------------------------

Comment By: tuxlife (tuxlife)
Date: 2008-08-05 04:09

Message:
Logged In: YES 
user_id=1404363
Originator: NO

http://fedoraproject.org/wiki/Security/Features#Compile_Time_Buffer_Checks_.28FORTIFY_SOURCE.29
:

CC compiler and GLIBC C library from Fedora Core 4 onwards has gained a
feature called "FORTIFY_SOURCE" that will detect and prevent a subset of
the buffer overflows before they can do damage. The idea behind
FORTIFY_SOURCE is relatively simple: there are cases where the compiler can
know the size of a buffer (if it's a fixed sized buffer on the stack, as in
the example, or if the buffer just came from a malloc() function call).
With a known buffer size, functions that operate on the buffer can make
sure the buffer will not overflow. FORTIFY_SOURCE in Fedora 8 has been
enhanced to cover C++ in addition to C, which prevents many security
exploits. 

References: 

http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02055.html

----------------------------------------------------------------------

Comment By: Thomas Guyot (dermoth)
Date: 2008-08-04 12:07

Message:
Logged In: YES 
user_id=375623
Originator: NO

Thanks for your feedback. I noticed this as well when the bug was reported
but I haven't had time to debug it yet.

I'm not sure that the number means, but it fails even with
-D_FORTIFY_SOURCE. Any more info on this flag would be nice...

----------------------------------------------------------------------

Comment By: tuxlife (tuxlife)
Date: 2008-08-04 10:53

Message:
Logged In: YES 
user_id=1404363
Originator: NO

I had the same problem with a self-RPM. 

The reason was following CFLAGS/CXXFLAGS/FFLAGS option:
-D_FORTIFY_SOURCE=2

Without this option does the plugin work.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=397597&aid=1999319&group_id=29880