author | Steven Luo <steven@steven676.net> | 2013-07-31 20:17:34 (GMT) |
---|---|---|
committer | Steven Luo <steven@steven676.net> | 2013-07-31 20:17:34 (GMT) |
commit | 3f2a90a33ce303046fce820f3934aefe23abb065 (patch) | |
tree | 6840fd012f54ad65ce9d092fac0e9ee089283531 | |
parent | ececd8c5a029f5f8c0069a2ec872d7b4a795edc0 (diff) | |
download | busybox-3f2a90a33ce303046fce820f3934aefe23abb065.zip busybox-3f2a90a33ce303046fce820f3934aefe23abb065.tar.gz busybox-3f2a90a33ce303046fce820f3934aefe23abb065.tar.bz2 |
mktemp: go back to using mktemp() for mktemp -u
The -u option to mktemp(1) is inherently unsafe, and the fact it exists
in the first place is probably a mistake. That said, it exists, and if
we want to support it, we should try to minimize the risk of using it.
The current code attempts to work around a build failure caused by
passing --fatal-warnings to the linker by using mkstemp() and unlinking
the resulting file, which eliminates the need for an attacker to guess
the filename (he can instead watch the temporary file directory using
inotify or some other similar mechanism). It's better instead to use
mktemp() (which at least forces the attacker to guess what "XXXXXX" was
replaced with) and tell the linker to live with the warning.
Change-Id: I01e0901db39821b7d1ce06145d1199ecd929e689
-rw-r--r-- | debianutils/mktemp.c | 12 |
1 files changed, 2 insertions, 10 deletions
diff --git a/debianutils/mktemp.c b/debianutils/mktemp.c index f9f067b..152bd95 100644 --- a/debianutils/mktemp.c +++ b/debianutils/mktemp.c @@ -93,17 +93,9 @@ int mktemp_main(int argc UNUSED_PARAM, char **argv) chp = concat_path_file(path, chp); if (opts & OPT_u) { -#ifdef __BIONIC__ - int fd = mkstemp(chp); - if (fd < 0) - goto error; - - /* unlink created file */ - close(fd); - unlink(chp); -#else chp = mktemp(chp); -#endif + if (chp[0] == '\0') + goto error; } else if (opts & OPT_d) { if (mkdtemp(chp) == NULL) goto error; |