[SRU] Buffer overflow in check_ntp_peer - Nagios can't check time servers in Intrepid

Confirmed, something gets caught by the stack smashing police.
Regression in intrepid, as it was working well in hardy.

It fails on line 264 on
if(read(conn, &req, SIZEOF_NTPCM(req)) == -1)

Upstream bug is:
http://sourceforge.net/tracker/?func=detail&atid=397597&aid=1999319&group_id=29880

It's closed by saying the bug is in _FORTIFY_SOURCE, as the author checked that the "read" call should not exceed allocated value.

"check_ntp" doesn't fail so it can be used as a workaround.

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

Thanks Jamie, this will be fixed soon in SVN. See the sourceforge tracker (Comment #2) for more details and updates.

FYI this bug is present in all version of check_ntp (in "plugins/" only) and check_ntp_peer (from which the code was taken), it's just that for some reasons it doesn't get triggered. The SIZEOF_NTPCM formula was broken in the very first versions of check_ntp (note though that very old versions of nagios-plugins shipped a Perl version of check_ntp, found in "plugins-scripts/").

Fix is in SVN r2086:
http://nagiosplug.svn.sourceforge.net/viewvc/nagiosplug?view=rev&revision=2086

Debdiff to fix this in the current development release

nagios-plugins (1.4.12-4ubuntu2) jaunty; urgency=low

  * Added 99_check_ntp_segfaults.dpatch: Fix for check_ntp and check_ntp_peer
    segfaults (LP: #291265)

This bug was fixed in the package nagios-plugins - 1.4.12-4ubuntu2

---------------
nagios-plugins (1.4.12-4ubuntu2) jaunty; urgency=low

  * Added 99_check_ntp_segfaults.dpatch: Fix for check_ntp and check_ntp_peer
    segfaults (LP: #291265)

 -- Thierry Carrez <email address hidden> Wed, 19 Nov 2008 16:44:27 +0000

Thank you for the job, mantainers and developers!

SRU report

Bug impact: Make Nagios ntp_check and ntp_check_peer plugins unusable under Intrepid. This is a regression, as this didn't trigger any error on previous releases.

Jaunty bugfix: fixed released in 1.4.12-4ubuntu2

Minimal patch: see below.

TEST CASE:
$ sudo apt-get install nagios-plugins-basic
With an affected version:
$ /usr/lib/nagios/plugins/check_ntp -H spork.qfe3.net -j 1
$ /usr/lib/nagios/plugins/check_ntp_peer -H spork.qfe3.net
both return a *** buffer overflow detected *** error stack
With a fixed version:
$ /usr/lib/nagios/plugins/check_ntp -H spork.qfe3.net -j 1
NTP OK: Décalage -0,006611943245 secs, jitter=0,000000|offset=-0,006612s;60,000000;120,000000; jitter=0,000000s;1,000000;10000,000000;0,000000
$ /usr/lib/nagios/plugins/check_ntp_peer -H spork.qfe3.net
NTP OK: Décalage -0,002 secs|offset=-0,002000s;60,000000;120,000000;

Regression potential:
The patch only affects those two plugins, and they are completely unusable without the fix.

Debdiff for Intrepid SRU

nagios-plugins (1.4.11-2ubuntu2.1) intrepid-proposed; urgency=low

  * Added 99_check_ntp_segfaults.dpatch: Fix for check_ntp and check_ntp_peer
    segfaults (LP: #291265)

Approved for SRU, please upload.

Accepted into intrepid-proposed, please test and give feedback here. Please see https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

check_ntp_peer works now:

Before stopping time server...

# /usr/lib/nagios/plugins/check_ntp_peer -H time.cefetrs.tche.br
NTP OK: Offset -0,002 secs|offset=-0,002000s;60,000000;120,000000;

After stopping time server...

# /usr/lib/nagios/plugins/check_ntp_peer -H time.cefetrs.tche.br
NTP CRITICAL: No response from NTP server

After starting time server again...

# /usr/lib/nagios/plugins/check_ntp_peer -H time.cefetrs.tche.br
NTP CRITICAL: Server not synchronized, Offset unknown|

# /usr/lib/nagios/plugins/check_ntp_peer -H time.cefetrs.tche.br
NTP OK: Offset -0,002 secs|offset=-0,002000s;60,000000;120,000000;

This bug was fixed in the package nagios-plugins - 1.4.11-2ubuntu2.1

---------------
nagios-plugins (1.4.11-2ubuntu2.1) intrepid-proposed; urgency=low

  * Added 99_check_ntp_segfaults.dpatch: Fix for check_ntp and check_ntp_peer
    segfaults (LP: #291265)

 -- Thierry Carrez <email address hidden> Thu, 20 Nov 2008 09:51:51 +0100

Copied to intrepid-updates.