[ntp:security] Re: Concerning a possible bug in the 'ntp' package
Danny Mayer
mayer at ntp.isc.org
Sat Sep 3 01:43:34 UTC 2005
David Wagner wrote:
> 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.
>
Yes, that part of the code has not changed much in ntpd/ntp_config.c.
However, it's part of ntpd and not part of ntp-keygen, so I really don't
understand how it is mentioned here rather than ntpd which does use it.
This part seems to be a flaw in the MOPS code.
> 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.
>
If they can do that they can do lots of other things on the box and I
think that ntp would be the least of an admin's problems, though I do
accept your point here.
> 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).
>
We will be looking to change our code anyway, though it happens be
unrelated to your report. The code is passing DNS requests and responses
between two processes.
> 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.
>
well ntpsim is not really an issue as it's a simulator and not a server
per se.
>
> 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;
> ...
> ntp_intres();
> and then might call abort_resolve(), which does:
> fclose(res_fp);
> (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:
> http://cert.uni-stuttgart.de/archive/bugtraq/2000/01/msg00066.html
>
> 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?
>
Well, while it's all possible, why would anyone who could break in
bother to do that. They basically would do a lot more damage elsewhere,
though I get your point.
Danny
> -- David
>
More information about the security
mailing list