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] moby/moby#38977 [2] containerd/containerd#3159 [3] containerd/containerd#4257 Signed-off-by: Kir Kolyshkin <[email protected]>
kolyshkin · Jun 23, 2020 · bd5e5a8 · bd5e5a8
1 parent 6e6306c
commit bd5e5a8
Show file tree

Hide file tree

Showing 2 changed files with 122 additions and 5 deletions.
diff --git 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
@@ -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
+		}
+	}
+
+}