From 7f698c89346acb5e5116736d25325a046652ba81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Wed, 28 Jun 2023 08:56:35 +0200 Subject: [PATCH] Don't panic on invalid security whitelist regexp Fixes #11176 --- config/security/securityConfig.go | 12 ++++++------ config/security/whitelist.go | 23 ++++++++++++++++++----- config/security/whitelist_test.go | 12 ++++++------ hugolib/integrationtest_builder.go | 3 ++- hugolib/testhelpers_test.go | 4 +++- markup/pandoc/convert_test.go | 4 +++- markup/rst/convert_test.go | 2 +- 7 files changed, 39 insertions(+), 21 deletions(-) diff --git a/config/security/securityConfig.go b/config/security/securityConfig.go index 5d0db2fb9..3d17b7a48 100644 --- a/config/security/securityConfig.go +++ b/config/security/securityConfig.go @@ -34,7 +34,7 @@ const securityConfigKey = "security" // DefaultConfig holds the default security policy. var DefaultConfig = Config{ Exec: Exec{ - Allow: NewWhitelist( + Allow: MustNewWhitelist( "^(dart-)?sass(-embedded)?$", // sass, dart-sass, dart-sass-embedded. "^go$", // for Go Modules "^npx$", // used by all Node tools (Babel, PostCSS). @@ -42,14 +42,14 @@ var DefaultConfig = Config{ ), // These have been tested to work with Hugo's external programs // on Windows, Linux and MacOS. - OsEnv: NewWhitelist(`(?i)^((HTTPS?|NO)_PROXY|PATH(EXT)?|APPDATA|TE?MP|TERM|GO\w+)$`), + OsEnv: MustNewWhitelist(`(?i)^((HTTPS?|NO)_PROXY|PATH(EXT)?|APPDATA|TE?MP|TERM|GO\w+)$`), }, Funcs: Funcs{ - Getenv: NewWhitelist("^HUGO_", "^CI$"), + Getenv: MustNewWhitelist("^HUGO_", "^CI$"), }, HTTP: HTTP{ - URLs: NewWhitelist(".*"), - Methods: NewWhitelist("(?i)GET|POST"), + URLs: MustNewWhitelist(".*"), + Methods: MustNewWhitelist("(?i)GET|POST"), }, } @@ -221,7 +221,7 @@ func stringSliceToWhitelistHook() mapstructure.DecodeHookFuncType { wl := types.ToStringSlicePreserveString(data) - return NewWhitelist(wl...), nil + return NewWhitelist(wl...) } } diff --git a/config/security/whitelist.go b/config/security/whitelist.go index 72a80da2e..92eb3102f 100644 --- a/config/security/whitelist.go +++ b/config/security/whitelist.go @@ -45,9 +45,9 @@ func (w Whitelist) MarshalJSON() ([]byte, error) { // NewWhitelist creates a new Whitelist from zero or more patterns. // An empty patterns list or a pattern with the value 'none' will create // a whitelist that will Accept none. -func NewWhitelist(patterns ...string) Whitelist { +func NewWhitelist(patterns ...string) (Whitelist, error) { if len(patterns) == 0 { - return Whitelist{acceptNone: true} + return Whitelist{acceptNone: true}, nil } var acceptSome bool @@ -68,7 +68,7 @@ func NewWhitelist(patterns ...string) Whitelist { if !acceptSome { return Whitelist{ acceptNone: true, - } + }, nil } var patternsr []*regexp.Regexp @@ -78,10 +78,23 @@ func NewWhitelist(patterns ...string) Whitelist { if p == "" { continue } - patternsr = append(patternsr, regexp.MustCompile(p)) + re, err := regexp.Compile(p) + if err != nil { + return Whitelist{}, fmt.Errorf("failed to compile whitelist pattern %q: %w", p, err) + } + patternsr = append(patternsr, re) } - return Whitelist{patterns: patternsr, patternsStrings: patternsStrings} + return Whitelist{patterns: patternsr, patternsStrings: patternsStrings}, nil +} + +// MustNewWhitelist creates a new Whitelist from zero or more patterns and panics on error. +func MustNewWhitelist(patterns ...string) Whitelist { + w, err := NewWhitelist(patterns...) + if err != nil { + panic(err) + } + return w } // Accept reports whether name is whitelisted. diff --git a/config/security/whitelist_test.go b/config/security/whitelist_test.go index 5c4196dff..89d1bc2b1 100644 --- a/config/security/whitelist_test.go +++ b/config/security/whitelist_test.go @@ -24,21 +24,21 @@ func TestWhitelist(t *testing.T) { c := qt.New(t) c.Run("none", func(c *qt.C) { - c.Assert(NewWhitelist("none", "foo").Accept("foo"), qt.IsFalse) - c.Assert(NewWhitelist().Accept("foo"), qt.IsFalse) - c.Assert(NewWhitelist("").Accept("foo"), qt.IsFalse) - c.Assert(NewWhitelist(" ", " ").Accept("foo"), qt.IsFalse) + c.Assert(MustNewWhitelist("none", "foo").Accept("foo"), qt.IsFalse) + c.Assert(MustNewWhitelist().Accept("foo"), qt.IsFalse) + c.Assert(MustNewWhitelist("").Accept("foo"), qt.IsFalse) + c.Assert(MustNewWhitelist(" ", " ").Accept("foo"), qt.IsFalse) c.Assert(Whitelist{}.Accept("foo"), qt.IsFalse) }) c.Run("One", func(c *qt.C) { - w := NewWhitelist("^foo.*") + w := MustNewWhitelist("^foo.*") c.Assert(w.Accept("foo"), qt.IsTrue) c.Assert(w.Accept("mfoo"), qt.IsFalse) }) c.Run("Multiple", func(c *qt.C) { - w := NewWhitelist("^foo.*", "^bar.*") + w := MustNewWhitelist("^foo.*", "^bar.*") c.Assert(w.Accept("foo"), qt.IsTrue) c.Assert(w.Accept("bar"), qt.IsTrue) c.Assert(w.Accept("mbar"), qt.IsFalse) diff --git a/hugolib/integrationtest_builder.go b/hugolib/integrationtest_builder.go index cb34cb28b..9c40fa7d0 100644 --- a/hugolib/integrationtest_builder.go +++ b/hugolib/integrationtest_builder.go @@ -381,7 +381,8 @@ func (s *IntegrationTestBuilder) initBuilder() error { s.Assert(os.Chdir(s.Cfg.WorkingDir), qt.IsNil) s.C.Cleanup(func() { os.Chdir(wd) }) sc := security.DefaultConfig - sc.Exec.Allow = security.NewWhitelist("npm") + sc.Exec.Allow, err = security.NewWhitelist("npm") + s.Assert(err, qt.IsNil) ex := hexec.New(sc) command, err := ex.New("npm", "install") s.Assert(err, qt.IsNil) diff --git a/hugolib/testhelpers_test.go b/hugolib/testhelpers_test.go index e1fd537a8..73920bd49 100644 --- a/hugolib/testhelpers_test.go +++ b/hugolib/testhelpers_test.go @@ -834,7 +834,9 @@ func (s *sitesBuilder) GetPageRel(p page.Page, ref string) page.Page { func (s *sitesBuilder) NpmInstall() hexec.Runner { sc := security.DefaultConfig - sc.Exec.Allow = security.NewWhitelist("npm") + var err error + sc.Exec.Allow, err = security.NewWhitelist("npm") + s.Assert(err, qt.IsNil) ex := hexec.New(sc) command, err := ex.New("npm", "install") s.Assert(err, qt.IsNil) diff --git a/markup/pandoc/convert_test.go b/markup/pandoc/convert_test.go index 6a1535946..dec30c410 100644 --- a/markup/pandoc/convert_test.go +++ b/markup/pandoc/convert_test.go @@ -31,7 +31,9 @@ func TestConvert(t *testing.T) { } c := qt.New(t) sc := security.DefaultConfig - sc.Exec.Allow = security.NewWhitelist("pandoc") + var err error + sc.Exec.Allow, err = security.NewWhitelist("pandoc") + c.Assert(err, qt.IsNil) p, err := Provider.New(converter.ProviderConfig{Exec: hexec.New(sc), Logger: loggers.NewDefault()}) c.Assert(err, qt.IsNil) conv, err := p.New(converter.DocumentContext{}) diff --git a/markup/rst/convert_test.go b/markup/rst/convert_test.go index 9e98d0405..1897e650f 100644 --- a/markup/rst/convert_test.go +++ b/markup/rst/convert_test.go @@ -31,7 +31,7 @@ func TestConvert(t *testing.T) { } c := qt.New(t) sc := security.DefaultConfig - sc.Exec.Allow = security.NewWhitelist("rst", "python") + sc.Exec.Allow = security.MustNewWhitelist("rst", "python") p, err := Provider.New( converter.ProviderConfig{