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

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

5 files changed, 73 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()
diff --git a/submodule_test.go b/submodule_test.go
index 7c97179..2c0a2ed 100644
--- a/submodule_test.go
+++ b/submodule_test.go
@@ -196,6 +196,21 @@ func (s *SubmoduleSuite) TestSubmodulesInit(c *C) {
 	}
 }
 
+func (s *SubmoduleSuite) TestGitSubmodulesSymlink(c *C) {
+	f, err := s.Worktree.Filesystem.Create("badfile")
+	c.Assert(err, IsNil)
+	defer f.Close()
+
+	err = s.Worktree.Filesystem.Remove(gitmodulesFile)
+	c.Assert(err, IsNil)
+
+	err = s.Worktree.Filesystem.Symlink("badfile", gitmodulesFile)
+	c.Assert(err, IsNil)
+
+	_, err = s.Worktree.Submodules()
+	c.Assert(err, Equals, ErrGitModulesSymlink)
+}
+
 func (s *SubmoduleSuite) TestSubmodulesStatus(c *C) {
 	sm, err := s.Worktree.Submodules()
 	c.Assert(err, IsNil)
diff --git a/worktree.go b/worktree.go
index ddf6fff..99b2cd1 100644
--- a/worktree.go
+++ b/worktree.go
@@ -28,6 +28,7 @@ var (
 	ErrWorktreeNotClean  = errors.New("worktree is not clean")
 	ErrSubmoduleNotFound = errors.New("submodule not found")
 	ErrUnstagedChanges   = errors.New("worktree contains unstaged changes")
+	ErrGitModulesSymlink = errors.New(gitmodulesFile + " is a symlink")
 )
 
 // Worktree represents a git worktree.
@@ -680,7 +681,18 @@ func (w *Worktree) newSubmodule(fromModules, fromConfig *config.Submodule) *Subm
 	return m
 }
 
+func (w *Worktree) isSymlink(path string) bool {
+	if s, err := w.Filesystem.Lstat(path); err == nil {
+		return s.Mode()&os.ModeSymlink != 0
+	}
+	return false
+}
+
 func (w *Worktree) readGitmodulesFile() (*config.Modules, error) {
+	if w.isSymlink(gitmodulesFile) {
+		return nil, ErrGitModulesSymlink
+	}
+
 	f, err := w.Filesystem.Open(gitmodulesFile)
 	if err != nil {
 		if os.IsNotExist(err) {