From bd5e5a83a911cc19969615d7d4864b0604e82af6 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 22 Jun 2020 17:29:51 -0700 Subject: [PATCH] mountinfo: fix path unescaping Moby PR #38977 [1] and containerd PR #3159 [2] added the handing of escape sequences in mountinfo paths (root and mountpoint fields). Unfortunately, it also broke the handling of paths containing double quotes, as it was pointed out in [3]. The solution is to stop using strconv.Unquote and write our own specialized function to deal with escape sequences. Unit tests added. [1] https://github.com/moby/moby/pull/38977 [2] https://github.com/containerd/containerd/pull/3159 [3] https://github.com/containerd/containerd/issues/4257 Signed-off-by: Kir Kolyshkin --- mountinfo/mountinfo_linux.go | 66 +++++++++++++++++++++++++++++-- mountinfo/mountinfo_linux_test.go | 61 +++++++++++++++++++++++++++- 2 files changed, 122 insertions(+), 5 deletions(-) diff --git a/mountinfo/mountinfo_linux.go b/mountinfo/mountinfo_linux.go index 2d630c8d..87f82241 100644 --- a/mountinfo/mountinfo_linux.go +++ b/mountinfo/mountinfo_linux.go @@ -71,9 +71,9 @@ func parseInfoFile(r io.Reader, filter FilterFunc) ([]*Info, error) { p := &Info{} // Fill in the fields that a filter might check - p.Mountpoint, err = strconv.Unquote(`"` + fields[4] + `"`) + p.Mountpoint, err = unescape(fields[4]) if err != nil { - return nil, fmt.Errorf("Parsing '%s' failed: unable to unquote mount point field: %w", fields[4], err) + return nil, fmt.Errorf("Parsing '%s' failed: mount point: %w", fields[4], err) } p.Fstype = fields[sepIdx+1] p.Source = fields[sepIdx+2] @@ -101,9 +101,9 @@ func parseInfoFile(r io.Reader, filter FilterFunc) ([]*Info, error) { p.Major, _ = strconv.Atoi(mm[0]) p.Minor, _ = strconv.Atoi(mm[1]) - p.Root, err = strconv.Unquote(`"` + fields[3] + `"`) + p.Root, err = unescape(fields[3]) if err != nil { - return nil, fmt.Errorf("Parsing '%s' failed: unable to unquote root field: %w", fields[3], err) + return nil, fmt.Errorf("Parsing '%s' failed: root: %w", fields[3], err) } p.Opts = fields[5] @@ -150,3 +150,61 @@ func PidMountInfo(pid int) ([]*Info, error) { return parseInfoFile(f, nil) } + +// A few specific characters in mountinfo path entries (root and mountpoint) +// are escaped using backslack followed by a character's ascii code in octal. +// +// space -- as \040 +// tab (aka \t) -- as \011 +// newline (aka \n) -- as \012 +// backslash (aka \\) -- as \134 +// +// This function converts path from mountinfo back, i.e. it unescapes the above sequences. +func unescape(path string) (string, error) { + // try to avoid copying + if strings.IndexByte(path, '\\') == -1 { + return path, nil + } + + // The following code is UTF-8 transparent as it only looks for some + // specific characters (backslach and 0..7) with values < utf8.RuneSelf, + // and everything else is passed through as is. + buf := make([]byte, len(path)) + bufLen := 0 + for i := 0; i < len(path); i++ { + if path[i] != '\\' { + buf[bufLen] = path[i] + bufLen++ + continue + } + s := path[i:] + if len(s) < 4 { + // too short + return "", fmt.Errorf("bad escape sequence %q: too short", s) + } + c := s[1] + switch c { + case '0', '1', '2', '3', '4', '5', '6', '7': + v := c - '0' + for j := 2; j < 4; j++ { // one digit already; two more + x := s[j] - '0' + if x < 0 || x > 7 { + return "", fmt.Errorf("bad escape sequence %q: not a digit", s[:3]) + } + v = (v << 3) | x + } + if v > 255 { + return "", fmt.Errorf("bad escape sequence %q: out of range" + s[:3]) + } + buf[bufLen] = v + bufLen++ + i += 3 + continue + default: + return "", fmt.Errorf("bad escape sequence %q: not a digit" + s[:3]) + + } + } + + return string(buf[:bufLen]), nil +} diff --git a/mountinfo/mountinfo_linux_test.go b/mountinfo/mountinfo_linux_test.go index 2441c684..7c060e2f 100644 --- a/mountinfo/mountinfo_linux_test.go +++ b/mountinfo/mountinfo_linux_test.go @@ -422,7 +422,8 @@ const ( 99 15 8:33 / /media/REMOVE\040ME rw,nosuid,nodev,relatime - fuseblk /dev/sdc1 rw,user_id=0,group_id=0,allow_other,blksize=4096` mountInfoWithSpaces = `486 28 252:1 / /mnt/foo\040bar rw,relatime shared:243 - ext4 /dev/vda1 rw,data=ordered -31 21 0:23 / /DATA/foo_bla_bla rw,relatime - cifs //foo/BLA\040BLA\040BLA/ rw,sec=ntlm,cache=loose,unc=\\foo\BLA BLA BLA,username=my_login,domain=mydomain.com,uid=12345678,forceuid,gid=12345678,forcegid,addr=10.1.30.10,file_mode=0755,dir_mode=0755,nounix,rsize=61440,wsize=65536,actimeo=1` +31 21 0:23 / /DATA/foo_bla_bla rw,relatime - cifs //foo/BLA\040BLA\040BLA/ rw,sec=ntlm,cache=loose,unc=\\foo\BLA BLA BLA,username=my_login,domain=mydomain.com,uid=12345678,forceuid,gid=12345678,forcegid,addr=10.1.30.10,file_mode=0755,dir_mode=0755,nounix,rsize=61440,wsize=65536,actimeo=1 +649 94 259:5 /tmp/newline\012tab\011space\040backslash\134quote1'quote2" /tmp/newline\012tab\011space\040backslash\134quote1'quote2" rw,relatime shared:47 - ext4 /dev/nvme0n1p5 rw,seclabel` ) func TestParseFedoraMountinfo(t *testing.T) { @@ -511,6 +512,21 @@ func TestParseMountinfoWithSpaces(t *testing.T) { Source: `//foo/BLA\040BLA\040BLA/`, VfsOpts: `rw,sec=ntlm,cache=loose,unc=\\foo\BLA`, }, + { + ID: 649, + Parent: 94, + Major: 259, + Minor: 5, + Root: `/tmp/newline +tab space backslash\quote1'quote2"`, + Mountpoint: `/tmp/newline +tab space backslash\quote1'quote2"`, + Opts: "rw,relatime", + Optional: "shared:47", + Fstype: "ext4", + Source: `/dev/nvme0n1p5`, + VfsOpts: `rw,seclabel`, + }, } if len(infos) != len(expected) { @@ -655,3 +671,46 @@ func TestParseMountinfoExtraCases(t *testing.T) { } } } + +func TestUnescape(t *testing.T) { + testCases := []struct { + input, output string + isErr bool + }{ + {"", "", false}, + {"/", "/", false}, + {"/some/longer/path", "/some/longer/path", false}, + {"/path\\040with\\040spaces", "/path with spaces", false}, + {"/path/with\\134backslash", "/path/with\\backslash", false}, + {"/tab\\011in/path", "/tab\tin/path", false}, + {`/path/"with'quotes`, `/path/"with'quotes`, false}, + {`/path/"with'quotes,\040space,\011tab`, `/path/"with'quotes, space, tab`, false}, + {`\12`, "", true}, + {`\134`, `\`, false}, + {`"'"'"'`, `"'"'"'`, false}, + {`/\1345`, `/\5`, false}, + {`/\12x`, "", true}, + {`\0`, "", true}, + {`\x`, "", true}, + {"\\\\", "", true}, + } + + for _, tc := range testCases { + res, err := unescape(tc.input) + if tc.isErr == true { + if err == nil { + t.Errorf("Input %q, want error, got nil", tc.input) + } + // no more checks + continue + } + if res != tc.output { + t.Errorf("Input %q, want %q, got %q", tc.input, tc.output, res) + } + if err != nil { + t.Errorf("Input %q, want nil, got error %v", tc.input, err) + continue + } + } + +}