From 8ceb0c18fc6778b662fff3517fec9aa7a05176a3 Mon Sep 17 00:00:00 2001 From: Christian Muehlhaeuser Date: Sun, 9 Jan 2022 11:29:05 +0100 Subject: [PATCH 1/5] fix: don't crash accessing invalid pathinfo When fs.Stat returns an error, pathinfo may be nil. In such situations the only safe response seems to be to return the error to the caller. Without this fix, accessing pathinfo.IsDir() below would lead to a crash dereferencing a nil pointer. This crash can be reproduced by trying to initialize a Git repo with an invalid path name. Also see: https://github.com/muesli/gitty/issues/36 --- repository.go | 3 +++ repository_test.go | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/repository.go b/repository.go index e8eb53f70..7292df627 100644 --- a/repository.go +++ b/repository.go @@ -280,6 +280,9 @@ func dotGitToOSFilesystems(path string, detect bool) (dot, wt billy.Filesystem, pathinfo, err := fs.Stat("/") if !os.IsNotExist(err) { + if pathinfo == nil { + return nil, nil, err + } if !pathinfo.IsDir() && detect { fs = osfs.New(filepath.Dir(path)) } diff --git a/repository_test.go b/repository_test.go index e284df8d6..5f542158e 100644 --- a/repository_test.go +++ b/repository_test.go @@ -2948,6 +2948,13 @@ func (s *RepositorySuite) TestBrokenMultipleShallowFetch(c *C) { c.Assert(err, IsNil) } +func TestDotGitToOSFilesystems(t *testing.T) { + _, _, err := dotGitToOSFilesystems("\000", false) + if err == nil { + t.Fatal("expected error, got nil") + } +} + func BenchmarkObjects(b *testing.B) { defer fixtures.Clean() From 4490e7a77d26b955342335d2fd1e6675a556bd50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1ximo=20Cuadros?= Date: Wed, 19 Jan 2022 14:57:11 +0100 Subject: [PATCH 2/5] Repository: idiomatic TestDotGitToOSFilesystemsInvalidPath --- repository_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/repository_test.go b/repository_test.go index 5f542158e..c6f2678ed 100644 --- a/repository_test.go +++ b/repository_test.go @@ -2948,11 +2948,9 @@ func (s *RepositorySuite) TestBrokenMultipleShallowFetch(c *C) { c.Assert(err, IsNil) } -func TestDotGitToOSFilesystems(t *testing.T) { +func TestDotGitToOSFilesystemsInvalidPath(t *testing.T) { _, _, err := dotGitToOSFilesystems("\000", false) - if err == nil { - t.Fatal("expected error, got nil") - } + c.Assert(r, NotNil) } func BenchmarkObjects(b *testing.B) { From d440824c8b7663a514208327c5e8c74471903295 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1ximo=20Cuadros?= Date: Wed, 19 Jan 2022 14:57:44 +0100 Subject: [PATCH 3/5] Repository: fix TestDotGitToOSFilesystemsInvalidPath typo --- repository_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repository_test.go b/repository_test.go index c6f2678ed..a7a292cf4 100644 --- a/repository_test.go +++ b/repository_test.go @@ -2950,7 +2950,7 @@ func (s *RepositorySuite) TestBrokenMultipleShallowFetch(c *C) { func TestDotGitToOSFilesystemsInvalidPath(t *testing.T) { _, _, err := dotGitToOSFilesystems("\000", false) - c.Assert(r, NotNil) + c.Assert(err, NotNil) } func BenchmarkObjects(b *testing.B) { From f0051a3535ddb5b056b675d893e34cddc1b2aa96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1ximo=20Cuadros?= Date: Wed, 19 Jan 2022 15:37:39 +0100 Subject: [PATCH 4/5] Repository: fix test --- repository_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repository_test.go b/repository_test.go index a7a292cf4..8fc54aaa7 100644 --- a/repository_test.go +++ b/repository_test.go @@ -2948,7 +2948,7 @@ func (s *RepositorySuite) TestBrokenMultipleShallowFetch(c *C) { c.Assert(err, IsNil) } -func TestDotGitToOSFilesystemsInvalidPath(t *testing.T) { +func (s *RepositorySuite) TestCreateTagLightweight(c *C) { _, _, err := dotGitToOSFilesystems("\000", false) c.Assert(err, NotNil) } From 4af1024db0ebe0d3a2e43971b3b26ac4148f1bb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1ximo=20Cuadros?= Date: Wed, 19 Jan 2022 15:43:04 +0100 Subject: [PATCH 5/5] Repository: fix TestDotGitToOSFilesystemsInvalidPath --- repository_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repository_test.go b/repository_test.go index 8fc54aaa7..7a9db151d 100644 --- a/repository_test.go +++ b/repository_test.go @@ -2948,7 +2948,7 @@ func (s *RepositorySuite) TestBrokenMultipleShallowFetch(c *C) { c.Assert(err, IsNil) } -func (s *RepositorySuite) TestCreateTagLightweight(c *C) { +func (s *RepositorySuite) TestDotGitToOSFilesystemsInvalidPath(c *C) { _, _, err := dotGitToOSFilesystems("\000", false) c.Assert(err, NotNil) }