Merge lp:~mterry/snappy/allow-more-dots into lp:~snappy-dev/snappy/snappy-moved-to-github

[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.

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.

[John Lenton (chipaca)]

Nice, thank you for this!

One small comment (inline).

review: Approve

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

458. By Michael Terry on 2015-05-15

Add comment

[Michael Terry (mterry)]

Added comment!

=== modified file 'clickdeb/deb.go'
--- clickdeb/deb.go	2015-05-07 07:46:32 +0000
+++ clickdeb/deb.go	2015-05-15 12:41:20 +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-05-07 07:46:32 +0000
+++ clickdeb/deb_test.go	2015-05-15 12:41:20 +0000
@@ -153,11 +153,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()