On Fri, Jun 27, 2014 at 1:30 PM, Morten Kvistgaard
<[email protected]> wrote:
>> Please try attached patch.
>
> The patch is working great.
There's a buglet there (cur_fd is leaking). Here's the fixed version.
diff -d -urpN busybox.8/networking/ftpd.c busybox.9/networking/ftpd.c
--- busybox.8/networking/ftpd.c 2014-06-26 12:07:45.000000000 +0200
+++ busybox.9/networking/ftpd.c 2014-06-27 12:35:21.000000000 +0200
@@ -623,13 +623,7 @@ popen_ls(const char *opt)
argv[0] = "ftpd";
argv[1] = opt; /* "-l" or "-1" */
-#if BB_MMU
argv[2] = "--";
-#else
- /* NOMMU ftpd ls helper chdirs to argv[2],
- * preventing peer from seeing real root. */
- argv[2] = xrealloc_getcwd_or_warn(NULL);
-#endif
argv[3] = G.ftp_arg;
argv[4] = NULL;
@@ -650,17 +644,10 @@ popen_ls(const char *opt)
/*fflush_all(); - so far we dont use stdio on output */
pid = BB_MMU ? xfork() : xvfork();
if (pid == 0) {
- /* child */
#if !BB_MMU
- /* On NOMMU, we want to execute a child - copy of ourself.
- * In chroot we usually can't do it. Thus we chdir
- * out of the chroot back to original root,
- * and (see later below) execute bb_busybox_exec_path
- * relative to current directory */
- if (fchdir(G.root_fd) != 0)
- _exit(127);
- /*close(G.root_fd); - close_on_exec_on() took care of this */
+ int cur_fd;
#endif
+ /* child */
/* NB: close _first_, then move fd! */
close(outfd.rd);
xmove_fd(outfd.wr, STDOUT_FILENO);
@@ -674,19 +661,26 @@ popen_ls(const char *opt)
/* memset(&G, 0, sizeof(G)); - ls_main does it */
exit(ls_main(ARRAY_SIZE(argv) - 1, (char**) argv));
#else
- /* + 1: we must use relative path here if in chroot.
- * For example, execv("/proc/self/exe") will fail, since
- * it looks for "/proc/self/exe" _relative to chroot!_ */
- execv(bb_busybox_exec_path + 1, (char**) argv);
+ cur_fd = xopen(".", O_RDONLY | O_DIRECTORY);
+ /* On NOMMU, we want to execute a child - copy of ourself
+ * in order to unblock parent after vfork.
+ * In chroot we usually can't re-exec. Thus we escape
+ * out of the chroot back to original root.
+ */
+ if (G.root_fd >= 0) {
+ if (fchdir(G.root_fd) != 0 || chroot(".") != 0)
+ _exit(127);
+ /*close(G.root_fd); - close_on_exec_on() took care of this */
+ }
+ /* Child expects directory to list on fd #3 */
+ xmove_fd(cur_fd, 3);
+ execv(bb_busybox_exec_path, (char**) argv);
_exit(127);
#endif
}
/* parent */
close(outfd.wr);
-#if !BB_MMU
- free((char*)argv[2]);
-#endif
return outfd.rd;
}
@@ -705,8 +699,6 @@ handle_dir_common(int opts)
if (!(opts & USE_CTRL_CONN) && !port_or_pasv_was_seen())
return; /* port_or_pasv_was_seen emitted error response */
- /* -n prevents user/groupname display,
- * which can be problematic in chroot */
ls_fd = popen_ls((opts & LONG_LISTING) ? "-l" : "-1");
ls_fp = xfdopen_for_read(ls_fd);
/* FIXME: filenames with embedded newlines are mishandled */
@@ -1139,11 +1131,10 @@ int ftpd_main(int argc UNUSED_PARAM, cha
opts = getopt32(argv, "l1vS" IF_FEATURE_FTP_WRITE("w") "t:T:", &G.timeout, &abs_timeout, &G.verbose, &verbose_S);
if (opts & (OPT_l|OPT_1)) {
/* Our secret backdoor to ls */
-/* TODO: pass -n? It prevents user/group resolution, which may not work in chroot anyway */
/* TODO: pass -A? It shows dot files */
/* TODO: pass --group-directories-first? would be nice, but ls doesn't do that yet */
- xchdir(argv[2]);
- argv[2] = (char*)"--";
+ if (fchdir(3) != 0)
+ _exit(127);
/* memset(&G, 0, sizeof(G)); - ls_main does it */
return ls_main(argc, argv);
}
@@ -1185,9 +1176,9 @@ int ftpd_main(int argc UNUSED_PARAM, cha
G.root_fd = xopen("/", O_RDONLY | O_DIRECTORY);
close_on_exec_on(G.root_fd);
#endif
-
- if (argv[optind]) {
- xchroot(argv[optind]);
+ argv += optind;
+ if (argv[0]) {
+ xchroot(argv[0]);
}
//umask(077); - admin can set umask before starting us
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
