From c93363fc8ad61ef14a359c7578d407038a30c296 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Sat, 26 Jan 2019 16:30:02 +0100 Subject: [PATCH] daemon: parse only first pid/uid/socket seen We were alerted that the socket peer credential code was susceptible to an attack allowing creating of root account from an otherwise unprivileged user. The attack is well explained in the bug report referenced below. The quick summary is that the code can be fooled into thinking user is root thanks to specifically crafted client-side socket name. As a quick fix, parse only the first occurrence of each field extracted from remote address. Fixes: https://bugs.launchpad.net/snapd/+bug/1813365 Signed-off-by: Zygmunt Krynicki --- daemon/ucrednet.go | 6 +++--- daemon/ucrednet_test.go | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/daemon/ucrednet.go b/daemon/ucrednet.go index 5e981e115..2dccef8da 100644 --- a/daemon/ucrednet.go +++ b/daemon/ucrednet.go @@ -40,20 +40,20 @@ func ucrednetGet(remoteAddr string) (pid uint32, uid uint32, socket string, err uid = ucrednetNobody for _, token := range strings.Split(remoteAddr, ";") { var v uint64 - if strings.HasPrefix(token, "pid=") { + if pid == ucrednetNoProcess && strings.HasPrefix(token, "pid=") { if v, err = strconv.ParseUint(token[4:], 10, 32); err == nil { pid = uint32(v) } else { break } - } else if strings.HasPrefix(token, "uid=") { + } else if uid == ucrednetNobody && strings.HasPrefix(token, "uid=") { if v, err = strconv.ParseUint(token[4:], 10, 32); err == nil { uid = uint32(v) } else { break } } - if strings.HasPrefix(token, "socket=") { + if socket == "" && strings.HasPrefix(token, "socket=") { socket = token[7:] } diff --git a/daemon/ucrednet_test.go b/daemon/ucrednet_test.go index 83511a845..81345fb5c 100644 --- a/daemon/ucrednet_test.go +++ b/daemon/ucrednet_test.go @@ -178,3 +178,11 @@ func (s *ucrednetSuite) TestGet(c *check.C) { c.Check(uid, check.Equals, uint32(42)) c.Check(socket, check.Equals, "/run/snap.socket") } + +func (s *ucrednetSuite) TestParsingRemoteAddr(c *check.C) { + pid, uid, socket, err := ucrednetGet("pid=5678;uid=1000;gid=1001;socket=/run/snap.socket;uid=0;socket=/crafted") + c.Check(pid, check.Equals, uint32(5678)) + c.Check(uid, check.Equals, uint32(1000)) + c.Check(socket, check.Equals, "/run/snap.socket") + c.Check(err, check.IsNil) +} -- 2.20.1