[ntp:security] Re: Concerning a possible bug in the 'ntp' package
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
> 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?
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.
> -- David
More information about the security