[ntp:security] Re: Concerning a possible bug in the 'ntp' package

David Wagner daw at cs.berkeley.edu
Fri Sep 2 03:35:27 UTC 2005

Thanks for your comments.  For what it's worth, we did do a manual
inspection of the code to check that the report looked plausible.  I
apologize again that we looked at such old code.

Since you are interested, I took the time to download a more recent
version (ntp-dev-4.2.0b-20050827) and take a look at it.  The relevant
code seems to be basically the same, although it has moved to
ntpd/ntp_config.c and util/ntp-keygen.c.

Based on a manual audit, it looks to me like there may be a
vulnerability here.  I have not confirmed this by trying to put
together an exploit, and it's certainly possible I'm missing something.
Let me try to show you what I see, and you can tell me whether it
is a problem or not.

1) On platforms that don't have mkstemp(), there may be a TOCTTOU
vulnerability in ntpd and ntpsim.  The ntpd and ntpsim programs
(ntpdmain() and ntpsim()) call getconfig().  On option CONFIG_BROADCAST,
getconfig() calls save_resolve().  On such platforms, save_resolve()
executes the following:
    (void) mktemp(res_file);
    res_fp = fopen(res_file, "w");
    ... fprintf(res_fp, ...); ...
There are multiple risks here.  One problem is that the filename
produced by mktemp() is guessable.  An adversary who guesses the
filename will be /tmp/ntpd123456 could set up a symlink
    /tmp/ntpd123456 -> some system file
and then when save_resolve() calls fopen(), the system file will
be overwritten.  If the system file is, say, /etc/shadow, this is
not good.

Another problem is that the call to mktemp() followed by fopen()
is not atomic().  An adversary can change the binding of the
/tmp/ntpd123456 file (or whatever it is called) in between the
mktemp() and the fopen(), so that the symlink points where to the
system file by the time you do a fopen().

The correct fix is to never use mktemp(), since it is insecure.
As the mktemp() man page says,

       Never  use  mktemp().  Some implementations follow BSD 4.3 and replace
       XXXXXX by the current process id and a single letter, so that at  most
       26  different  names can be returned.  Since on the one hand the names
       are easy to guess, and on the other hand there is a race between test-
       ing  whether  the  name  exists  and  opening  the  file, every use of
       mktemp() is a security risk.  The race is avoided by mkstemp(3).

Potential impact: Running ntpd and ntpsim as root may cause
arbitrary files on the system to be corrupted or overwritten.
Even running them as non-root users may be unsafe.

2) I was also looking at the use of mkstemp(), and I'm worried about
the re-use of the mkstemp() filename, which is in general not safe.
Let me show you what I'm concerned about.  After getconfig() calls
save_resolve(), it may call do_resolve_internal().  On platforms that
do have mkstemp(), save_resolve() executes this code:
    strcpy(res_file, RES_TEMPFILE);
    ... fd = mkstemp(res_file) ...
    res_fp = fopen(fd, "r+");
    ... write to fd ....
Also, do_resolve_internal() executes
    req_file = res_file;
and then might call abort_resolve(), which does:
    (void) unlink(res_file);
Also, ntp_intres() executes this:
    ... in = fopen(req_file, "r") ...
    readconf(in, req_file);
with a comment nearby saying "this is bogus" (?).

A general comment: Using the file descriptor produced by mkstemp() is
safe, but re-using the filename produced by mkstemp() in another system
call is generally a rather fishy thing to do.  The problem is that the
state of the filesystem can change between the mkstemp() and the
subsequent use of that filename, so the filename might point to something
different than what you think when you later use it.  For instance, see
the following background on cryogenic sleep attacks, which describes one
way that an attacker can change the filename produced by mkstemp() into
a symlink that points to some arbitrary other file:

2a) An attacker might be able to arrange that you call abort_resolve()
and thus call unlink() on a filename /tmp/ntpd123456 that is a symlink
to some system file.  However, unlink() doesn't follow symlinks that occur
in the last component of the filename, and RES_TEMPFILE is hardcoded to
"/tmp/ntpdXXXXXX", so there is no way that a symlink could appear anywhere
other than the last component of the filename.  Assuming /tmp has its
sticky bit set (which it darn well better!), I can't see any risk here.

2b) The code in ntp_intres() looks more fishy, since it seems to be
reading in config file information from the filename constructed by
mkstemp().  But consider: What if /tmp/ntpd123456 disappears in a
cryogenic sleep attack, then an attacker creates a new file with the
same filename and with malicious contents?  Then ntp_intres() can be
tricked into reading config file information supplied the adversary.
Can this be used to compromise the rest of the program or cause the
program to misbehave or produce wrong results?  I don't know.  I guess
it depends on how the config file information is used and whether it
is trusted in any way.  I don't know enough about ntp to evaluate whether
this is a potential issue.

Any thoughts on the above analysis?

-- David

> David A Wagner wrote:
> > 
> > ------------------------------------------------------------------------
> > 
> > Subject:
> > Concerning a possible bug in the 'ntp' package
> > From:
> > Ben Schwarz <bschwarz at EECS.berkeley.EDU>
> > Date:
> > Mon, 29 Aug 2005 15:07:27 -0700 (PDT)
> > To:
> > bugs at ntp.org
> > 
> > To:
> > bugs at ntp.org
> > CC:
> > David A Wagner <daw at EECS.berkeley.EDU>
> > 
> > ----------------------------
> > 
> > URL with program traces for this package:
> > https://taverner.cs.berkeley.edu/traces/tmpfile/ntp-4.1.2-0.rc1.2/HTMLtrace/ 
> > 
> > 
> > Programs with bugs:
> > ntp-genkeys (ntp_config.c line 2088)
> > 
> > We believe this program re-uses the template from mkstemp(),
> > and suspect there may be a race condition with another system call.
> > 
> I reviewed this report. The are a large number of erroneous assumptions
> made in this report. I will enumerate the obvious ones:
> 1) ntp 4.1.2 is ancient and hasn't been shipped by the NTP project for
> years.
> 2) Never use a release candidate (rc1.2) for testing. That's not what
> gets shipped.
> 3) The NTP project has no control over what Redhat ships. All such
> problem reports should go first to the vendor.
> 4) There is no program named ntp-genkeys in ntp. There IS a program
> named ntp-keygen but this is a command-line-only app that generates keys
> and is never used as a server app.
> 5) ntp_config.c is not used by ntp-keygen, but is a part of ntpd which
> is a server app.
> 6) automatic generation of a bug report should be followed by a manual
> analysis of the problem to ensure that it's accurate. The results above
> show that it was wildly off and raises questions of the methodology used
> to analyze the code.
> 7) You finding of mkstemp() is not followed up by an analysis of how and
> why it is used. Just because it's being used does not indicate a
> problem. Please send an analysis of why it is incorrect to use this
> function in the context of the application and function calling it.
> Danny
> NTP Public Services Project

More information about the security mailing list