diff mbox

[v2] netns: avoid directory traversal (was: ip netns: Make sure netns name is sane)

Message ID 20170719223632.4124-1-mcroce@redhat.com
State Accepted, archived
Delegated to: stephen hemminger
Headers show

Commit Message

Matteo Croce July 19, 2017, 10:36 p.m. UTC
v2: reword commit message

ip netns keeps track of created namespaces with bind mounts named
/var/run/netns/<namespace>. No input sanitization is done, allowing creation and
deletion of files relatives to /var/run/netns or, if the path is non existent or
invalid, allows to create "untracked" namespaces (invisible to the tool).

This commit denies creation or deletion of namespaces with names contaning
"/" or matching exactly "." or "..".

Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
 ip/ipnetns.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Phil Sutter July 20, 2017, 7:44 a.m. UTC | #1
Hi Matteo,

I don't think git-am understands the subject change semantics you use
here, so please drop the '(was: ...)' part from it.

On Thu, Jul 20, 2017 at 12:36:32AM +0200, Matteo Croce wrote:
> v2: reword commit message

In my opinion, this belongs to patch meta info (below the three-dashes
line), not the commit message.

Please put the maintainer in Cc when submitting patches. It helps them
to keep track of it. This counts for all upstream projects, not just
iproute2.

Thanks, Phil
Stephen Hemminger July 21, 2017, 12:25 a.m. UTC | #2
On Thu, 20 Jul 2017 00:36:32 +0200
Matteo Croce <mcroce@redhat.com> wrote:

> v2: reword commit message
> 
> ip netns keeps track of created namespaces with bind mounts named
> /var/run/netns/<namespace>. No input sanitization is done, allowing creation and
> deletion of files relatives to /var/run/netns or, if the path is non existent or
> invalid, allows to create "untracked" namespaces (invisible to the tool).
> 
> This commit denies creation or deletion of namespaces with names contaning
> "/" or matching exactly "." or "..".
> 
> Signed-off-by: Matteo Croce <mcroce@redhat.com>

Applied. I manually edited the commit description to remove the (was...)
thanks Matteo.
diff mbox

Patch

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 0b0378ab..42549944 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -766,6 +766,11 @@  static int netns_monitor(int argc, char **argv)
 	return 0;
 }
 
+static int invalid_name(const char *name)
+{
+	return strchr(name, '/') || !strcmp(name, ".") || !strcmp(name, "..");
+}
+
 int do_netns(int argc, char **argv)
 {
 	netns_nsid_socket_init();
@@ -775,6 +780,11 @@  int do_netns(int argc, char **argv)
 		return netns_list(0, NULL);
 	}
 
+	if (argc > 1 && invalid_name(argv[1])) {
+		fprintf(stderr, "Invalid netns name \"%s\"\n", argv[1]);
+		exit(-1);
+	}
+
 	if ((matches(*argv, "list") == 0) || (matches(*argv, "show") == 0) ||
 	    (matches(*argv, "lst") == 0)) {
 		netns_map_init();