Merge pull request #848 from josephvusich/fix/cve-2018-11235

config: modules, worktree: Submodule fixes for CVE-2018-11235

3 files changed, 46 insertions, 12 deletions

diff --git a/config/config.go b/config/config.go
index c730015..ce6506d 100644
--- a/config/config.go
+++ b/config/config.go
@@ -135,7 +135,7 @@ func (c *Config) Unmarshal(b []byte) error {
 	if err := c.unmarshalPack(); err != nil {
 		return err
 	}
-	c.unmarshalSubmodules()
+	unmarshalSubmodules(c.Raw, c.Submodules)
 
 	if err := c.unmarshalBranches(); err != nil {
 		return err
@@ -182,13 +182,17 @@ func (c *Config) unmarshalRemotes() error {
 	return nil
 }
 
-func (c *Config) unmarshalSubmodules() {
-	s := c.Raw.Section(submoduleSection)
+func unmarshalSubmodules(fc *format.Config, submodules map[string]*Submodule) {
+	s := fc.Section(submoduleSection)
 	for _, sub := range s.Subsections {
 		m := &Submodule{}
 		m.unmarshal(sub)
 
-		c.Submodules[m.Name] = m
+		if m.Validate() == ErrModuleBadPath {
+			continue
+		}
+
+		submodules[m.Name] = m
 	}
 }
 
diff --git a/config/modules.go b/config/modules.go
index b208984..90758d9 100644
--- a/config/modules.go
+++ b/config/modules.go
@@ -3,6 +3,7 @@ package config
 import (
 	"bytes"
 	"errors"
+	"regexp"
 
 	format "gopkg.in/src-d/go-git.v4/plumbing/format/config"
 )
@@ -10,6 +11,12 @@ import (
 var (
 	ErrModuleEmptyURL  = errors.New("module config: empty URL")
 	ErrModuleEmptyPath = errors.New("module config: empty path")
+	ErrModuleBadPath   = errors.New("submodule has an invalid path")
+)
+
+var (
+	// Matches module paths with dotdot ".." components.
+	dotdotPath = regexp.MustCompile(`(^|[/\\])\.\.([/\\]|$)`)
 )
 
 // Modules defines the submodules properties, represents a .gitmodules file
@@ -44,14 +51,7 @@ func (m *Modules) Unmarshal(b []byte) error {
 		return err
 	}
 
-	s := m.raw.Section(submoduleSection)
-	for _, sub := range s.Subsections {
-		mod := &Submodule{}
-		mod.unmarshal(sub)
-
-		m.Submodules[mod.Path] = mod
-	}
-
+	unmarshalSubmodules(m.raw, m.Submodules)
 	return nil
 }
 
@@ -102,6 +102,10 @@ func (m *Submodule) Validate() error {
 		return ErrModuleEmptyURL
 	}
 
+	if dotdotPath.MatchString(m.Path) {
+		return ErrModuleBadPath
+	}
+
 	return nil
 }
 
diff --git a/config/modules_test.go b/config/modules_test.go
index 36cd93f..8e10d70 100644
--- a/config/modules_test.go
+++ b/config/modules_test.go
@@ -11,6 +11,29 @@ func (s *ModulesSuite) TestValidateMissingURL(c *C) {
 	c.Assert(m.Validate(), Equals, ErrModuleEmptyURL)
 }
 
+func (s *ModulesSuite) TestValidateBadPath(c *C) {
+	input := []string{
+		`..`,
+		`../`,
+		`../bar`,
+
+		`/..`,
+		`/../bar`,
+
+		`foo/..`,
+		`foo/../`,
+		`foo/../bar`,
+	}
+
+	for _, p := range input {
+		m := &Submodule{
+			Path: p,
+			URL:  "https://example.com/",
+		}
+		c.Assert(m.Validate(), Equals, ErrModuleBadPath)
+	}
+}
+
 func (s *ModulesSuite) TestValidateMissingName(c *C) {
 	m := &Submodule{URL: "bar"}
 	c.Assert(m.Validate(), Equals, ErrModuleEmptyPath)
@@ -39,6 +62,9 @@ func (s *ModulesSuite) TestUnmarshall(c *C) {
         path = foo/bar
         url = https://github.com/foo/bar.git
 		branch = dev
+[submodule "suspicious"]
+        path = ../../foo/bar
+        url = https://github.com/foo/bar.git
 `)
 
 	cfg := NewModules()