Merge lp:~mterry/snappy/allow-more-dots-15.04 into lp:~snappy-dev/snappy/15.04-deprecated

[Michael Terry]

Allow ".." in file paths as long as it isn't an entire element. (i.e. allow "hello..you" or "...")

We call filepath.Clean right before our ".." check. So the only instances of ".." will be in the first element. That lets us simplify the check a lot.

This is a backport of r457 from trunk.

Allow ".." in file paths as long as it isn't an entire element. (i.e. allow "hello..you" or "...")

We call filepath.Clean right before our ".." check. So the only instances of ".." will be in the first element. That lets us simplify the check a lot.

This is a backport of r457 from trunk.

[Sergio Schvezov (sergiusens)]

Do you mind adding a backport comment to the commit message for traceability since it seems to be lost with bzr.

Something like

bzr merge -c 457 lp:snappy or "changes from lp:~mterry/snappy/allow-more-dots"

Looks good

[lp:~mterry/snappy/allow-more-dots-15.04]

447. By Michael Terry on 2015-06-03

Backport r457 from trunk, which allows .. in file paths as long as it isn't an entire element.

[Michael Terry (mterry)]

OK try now. I used merge instead of diff/patch and mentioned the revision in the commit.

[John Lenton (chipaca)]

review: Approve

=== modified file 'clickdeb/deb.go'
--- clickdeb/deb.go	2015-04-23 11:12:32 +0000
+++ clickdeb/deb.go	2015-06-03 19:12:35 +0000
@@ -95,7 +95,9 @@
 // - no relative path allowed to prevent writing outside of the parent dir
 func clickVerifyContentFn(path string) (string, error) {
 	path = filepath.Clean(path)
-	if strings.Contains(path, "..") {
+	// Clean() will remove any internal ".." elements, except if it's a relative
+	// path and the ".." element is the first one.  So let's check for that:
+	if path == ".." || strings.HasPrefix(path, "../") {
 		return "", ErrSnapInvalidContent
 	}
 

=== modified file 'clickdeb/deb_test.go'
--- clickdeb/deb_test.go	2015-04-13 20:58:56 +0000
+++ clickdeb/deb_test.go	2015-06-03 19:12:35 +0000
@@ -152,11 +152,22 @@
 	c.Assert(newPath, Equals, "foo/baz")
 }
 
+func (s *ClickDebTestSuite) TestClickVerifyContentFnThreeDotsOk(c *C) {
+	newPath, err := clickVerifyContentFn(".../foo")
+	c.Assert(err, IsNil)
+	c.Assert(newPath, Equals, ".../foo")
+}
+
 func (s *ClickDebTestSuite) TestClickVerifyContentFnNotOk(c *C) {
 	_, err := clickVerifyContentFn("./foo/../../baz")
 	c.Assert(err, Equals, ErrSnapInvalidContent)
 }
 
+func (s *ClickDebTestSuite) TestClickVerifyContentFnJustTwoDotsNotOk(c *C) {
+	_, err := clickVerifyContentFn("..")
+	c.Assert(err, Equals, ErrSnapInvalidContent)
+}
+
 func (s *ClickDebTestSuite) TestTarCreate(c *C) {
 	// setup
 	builddir := c.MkDir()