From b74bc6268c9bdfac420be12472ef99fda0c02fd5 Mon Sep 17 00:00:00 2001 From: Xe Iaso Date: Fri, 8 May 2026 18:39:20 -0400 Subject: [PATCH] fix: patch GHSA-6wcg-mqvh-fcvg PR https://github.com/TecharoHQ/anubis/pull/1015 added the ability for reverse proxies using Anubis in subrequest auth mode to look at the path of a request as there are many rules in the wild that rely on checking the path. This is how access to things like robots.txt or anything in the .well-known directory is unaffected by Anubis. However this logic was also enabled for non-subrequest deployments of Anubis, meaning that a specially crafted request could include a /.well-known/ path in it and then get around Anubis with little effort. This fix gates the logic behind a new plumbed variable named subrequestMode that only fires when Anubis is running in subrequest auth mode. This properly contains that workaround so that the logic does not fire in most deployments. Signed-off-by: Xe Iaso --- cmd/anubis/main.go | 2 +- docs/docs/CHANGELOG.md | 1 + internal/test/playwright_test.go | 2 +- lib/anubis_test.go | 4 +- lib/config.go | 4 +- lib/config_test.go | 8 +-- lib/policy/checker.go | 19 +++--- lib/policy/checker_test.go | 109 ++++++++++++++++++++++++++++++- lib/policy/policy.go | 4 +- lib/policy/policy_test.go | 8 +-- 10 files changed, 135 insertions(+), 26 deletions(-) diff --git a/cmd/anubis/main.go b/cmd/anubis/main.go index d6ce5000..b1a906ab 100644 --- a/cmd/anubis/main.go +++ b/cmd/anubis/main.go @@ -259,7 +259,7 @@ func main() { } lg.Info("loading policy file", "fname", *policyFname) - policy, err := libanubis.LoadPoliciesOrDefault(ctx, *policyFname, *challengeDifficulty, *slogLevel) + policy, err := libanubis.LoadPoliciesOrDefault(ctx, *policyFname, *challengeDifficulty, *slogLevel, strings.TrimSpace(*target) == "") if err != nil { log.Fatalf("can't parse policy file: %v", err) } diff --git a/docs/docs/CHANGELOG.md b/docs/docs/CHANGELOG.md index 2175ad86..ba4bec51 100644 --- a/docs/docs/CHANGELOG.md +++ b/docs/docs/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +- Patch [GHSA-6wcg-mqvh-fcvg](https://github.com/TecharoHQ/anubis/security/advisories/GHSA-6wcg-mqvh-fcvg) by containing subrequest logic to Anubis instances in subrequest mode. - Move metrics server configuration to [the policy file](./admin/policies.mdx#metrics-server). - Expose [pprof endpoints](https://pkg.go.dev/net/http/pprof) on the metrics listener to enable profiling Anubis in production. - fix: prevent nil pointer panic in challenge validation when threshold rules match during PassChallenge (#1463) diff --git a/internal/test/playwright_test.go b/internal/test/playwright_test.go index 4d6355ba..6961dc02 100644 --- a/internal/test/playwright_test.go +++ b/internal/test/playwright_test.go @@ -595,7 +595,7 @@ func spawnAnubisWithOptions(t *testing.T, basePrefix string) string { fmt.Fprintf(w, "%d", time.Now().Unix()) }) - policy, err := libanubis.LoadPoliciesOrDefault(t.Context(), "", anubis.DefaultDifficulty, "info") + policy, err := libanubis.LoadPoliciesOrDefault(t.Context(), "", anubis.DefaultDifficulty, "info", false) if err != nil { t.Fatal(err) } diff --git a/lib/anubis_test.go b/lib/anubis_test.go index 7c0f87d3..3ea07199 100644 --- a/lib/anubis_test.go +++ b/lib/anubis_test.go @@ -58,7 +58,7 @@ func loadPolicies(t *testing.T, fname string, difficulty int) *policy.ParsedConf t.Logf("loading policy file: %s", fname) - anubisPolicy, err := LoadPoliciesOrDefault(ctx, fname, difficulty, "info") + anubisPolicy, err := LoadPoliciesOrDefault(ctx, fname, difficulty, "info", false) if err != nil { t.Fatal(err) } @@ -250,7 +250,7 @@ func TestLoadPolicies(t *testing.T) { } defer fin.Close() - if _, err := policy.ParseConfig(t.Context(), fin, fname, 4, "info"); err != nil { + if _, err := policy.ParseConfig(t.Context(), fin, fname, 4, "info", false); err != nil { t.Fatal(err) } }) diff --git a/lib/config.go b/lib/config.go index e8b7626d..f577ecf7 100644 --- a/lib/config.go +++ b/lib/config.go @@ -55,7 +55,7 @@ type Options struct { DifficultyInJWT bool } -func LoadPoliciesOrDefault(ctx context.Context, fname string, defaultDifficulty int, logLevel string) (*policy.ParsedConfig, error) { +func LoadPoliciesOrDefault(ctx context.Context, fname string, defaultDifficulty int, logLevel string, subrequestMode bool) (*policy.ParsedConfig, error) { var fin io.ReadCloser var err error @@ -79,7 +79,7 @@ func LoadPoliciesOrDefault(ctx context.Context, fname string, defaultDifficulty } }(fin) - anubisPolicy, err := policy.ParseConfig(ctx, fin, fname, defaultDifficulty, logLevel) + anubisPolicy, err := policy.ParseConfig(ctx, fin, fname, defaultDifficulty, logLevel, subrequestMode) if err != nil { return nil, fmt.Errorf("can't parse policy file %s: %w", fname, err) } diff --git a/lib/config_test.go b/lib/config_test.go index 99aab736..3fdc3259 100644 --- a/lib/config_test.go +++ b/lib/config_test.go @@ -12,7 +12,7 @@ import ( ) func TestInvalidChallengeMethod(t *testing.T) { - if _, err := LoadPoliciesOrDefault(t.Context(), "testdata/invalid-challenge-method.yaml", 4, "info"); !errors.Is(err, policy.ErrChallengeRuleHasWrongAlgorithm) { + if _, err := LoadPoliciesOrDefault(t.Context(), "testdata/invalid-challenge-method.yaml", 4, "info", false); !errors.Is(err, policy.ErrChallengeRuleHasWrongAlgorithm) { t.Fatalf("wanted error %v but got %v", policy.ErrChallengeRuleHasWrongAlgorithm, err) } } @@ -25,7 +25,7 @@ func TestBadConfigs(t *testing.T) { for _, st := range finfos { t.Run(st.Name(), func(t *testing.T) { - if _, err := LoadPoliciesOrDefault(t.Context(), filepath.Join("config", "testdata", "bad", st.Name()), anubis.DefaultDifficulty, "info"); err == nil { + if _, err := LoadPoliciesOrDefault(t.Context(), filepath.Join("config", "testdata", "bad", st.Name()), anubis.DefaultDifficulty, "info", false); err == nil { t.Fatal(err) } else { t.Log(err) @@ -44,13 +44,13 @@ func TestGoodConfigs(t *testing.T) { t.Run(st.Name(), func(t *testing.T) { t.Run("with-thoth", func(t *testing.T) { ctx := thothmock.WithMockThoth(t) - if _, err := LoadPoliciesOrDefault(ctx, filepath.Join("config", "testdata", "good", st.Name()), anubis.DefaultDifficulty, "info"); err != nil { + if _, err := LoadPoliciesOrDefault(ctx, filepath.Join("config", "testdata", "good", st.Name()), anubis.DefaultDifficulty, "info", false); err != nil { t.Fatal(err) } }) t.Run("without-thoth", func(t *testing.T) { - if _, err := LoadPoliciesOrDefault(t.Context(), filepath.Join("config", "testdata", "good", st.Name()), anubis.DefaultDifficulty, "info"); err != nil { + if _, err := LoadPoliciesOrDefault(t.Context(), filepath.Join("config", "testdata", "good", st.Name()), anubis.DefaultDifficulty, "info", false); err != nil { t.Fatal(err) } }) diff --git a/lib/policy/checker.go b/lib/policy/checker.go index 8b88a5be..38003164 100644 --- a/lib/policy/checker.go +++ b/lib/policy/checker.go @@ -94,23 +94,26 @@ func (hmc *HeaderMatchesChecker) Hash() string { } type PathChecker struct { - regexp *regexp.Regexp - hash string + regexp *regexp.Regexp + hash string + subRequestMode bool } -func NewPathChecker(rexStr string) (checker.Impl, error) { +func NewPathChecker(rexStr string, subrequestMode bool) (checker.Impl, error) { rex, err := regexp.Compile(strings.TrimSpace(rexStr)) if err != nil { return nil, fmt.Errorf("%w: regex %s failed parse: %w", ErrMisconfiguration, rexStr, err) } - return &PathChecker{rex, internal.FastHash(rexStr)}, nil + return &PathChecker{rex, internal.FastHash(rexStr), subrequestMode}, nil } func (pc *PathChecker) Check(r *http.Request) (bool, error) { - originalUrl := r.Header.Get("X-Original-URI") - if originalUrl != "" { - if pc.regexp.MatchString(originalUrl) { - return true, nil + if pc.subRequestMode { + originalUrl := r.Header.Get("X-Original-URI") + if originalUrl != "" { + if pc.regexp.MatchString(originalUrl) { + return true, nil + } } } diff --git a/lib/policy/checker_test.go b/lib/policy/checker_test.go index ce27a782..59ee03fb 100644 --- a/lib/policy/checker_test.go +++ b/lib/policy/checker_test.go @@ -272,8 +272,8 @@ func TestPathChecker_XOriginalURI(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create the PathChecker - pc, err := NewPathChecker(tt.regex) + // Create the PathChecker in subrequest mode so X-Original-URI is honored. + pc, err := NewPathChecker(tt.regex, true) if err != nil { if !tt.expectError { t.Fatalf("NewPathChecker() unexpected error: %v", err) @@ -305,3 +305,108 @@ func TestPathChecker_XOriginalURI(t *testing.T) { }) } } + +// TestPathChecker_GHSA_6wcg_mqvh_fcvg is a regression test for +// https://github.com/TecharoHQ/anubis/security/advisories/GHSA-6wcg-mqvh-fcvg. +// +// PR https://github.com/TecharoHQ/anubis/pull/1015 added the ability for +// reverse proxies using Anubis in subrequest auth mode to look at the path +// of a request as there are many rules in the wild that rely on checking +// the path. This is how access to things like robots.txt or anything in the +// .well-known directory is unaffected by Anubis. +// +// However this logic was also enabled for non-subrequest deployments of Anubis, +// meaning that a specially crafted request could include a /.well-known/ +// path in it and then get around Anubis with little effort. +// +// This fix gates the logic behind a new plumbed variable named subrequestMode +// that only fires when Anubis is running in subrequest auth mode. This +// properly contains that workaround so that the logic does not fire in +// most deployments. +func TestPathChecker_GHSA_6wcg_mqvh_fcvg(t *testing.T) { + tests := []struct { + name string + regex string + urlPath string + xOriginalURI string + subRequestMode bool + want bool + }{ + { + name: "default mode ignores spoofed X-Original-URI when real path matches", + regex: "^/admin/.*", + urlPath: "/admin/secret", + xOriginalURI: "/public/index", + subRequestMode: false, + want: true, + }, + { + name: "default mode ignores spoofed X-Original-URI when real path does not match", + regex: "^/admin/.*", + urlPath: "/public/index", + xOriginalURI: "/admin/secret", + subRequestMode: false, + want: false, + }, + { + name: "default mode without X-Original-URI matches real path", + regex: "^/admin/.*", + urlPath: "/admin/dashboard", + xOriginalURI: "", + subRequestMode: false, + want: true, + }, + { + name: "subrequest mode honors X-Original-URI", + regex: "^/admin/.*", + urlPath: "/auth", + xOriginalURI: "/admin/secret", + subRequestMode: true, + want: true, + }, + { + name: "subrequest mode falls back to URL.Path when X-Original-URI does not match", + regex: "^/admin/.*", + urlPath: "/admin/dashboard", + xOriginalURI: "/public/index", + subRequestMode: true, + want: true, + }, + { + name: "subrequest mode with empty X-Original-URI uses URL.Path", + regex: "^/admin/.*", + urlPath: "/admin/dashboard", + xOriginalURI: "", + subRequestMode: true, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pc, err := NewPathChecker(tt.regex, tt.subRequestMode) + if err != nil { + t.Fatalf("NewPathChecker(%q, %v) returned error: %v", tt.regex, tt.subRequestMode, err) + } + + req, err := http.NewRequest(http.MethodGet, "http://example.com"+tt.urlPath, nil) + if err != nil { + t.Fatalf("http.NewRequest: %v", err) + } + + if tt.xOriginalURI != "" { + req.Header.Set("X-Original-URI", tt.xOriginalURI) + } + + got, err := pc.Check(req) + if err != nil { + t.Fatalf("Check() unexpected error: %v", err) + } + + if got != tt.want { + t.Errorf("Check() = %v, want %v (subRequestMode=%v, urlPath=%q, X-Original-URI=%q)", + got, tt.want, tt.subRequestMode, tt.urlPath, tt.xOriginalURI) + } + }) + } +} diff --git a/lib/policy/policy.go b/lib/policy/policy.go index fd38079f..3889dc8e 100644 --- a/lib/policy/policy.go +++ b/lib/policy/policy.go @@ -60,7 +60,7 @@ func newParsedConfig(orig *config.Config) *ParsedConfig { } } -func ParseConfig(ctx context.Context, fin io.Reader, fname string, defaultDifficulty int, logLevel string) (*ParsedConfig, error) { +func ParseConfig(ctx context.Context, fin io.Reader, fname string, defaultDifficulty int, logLevel string, subrequestMode bool) (*ParsedConfig, error) { c, err := config.Load(fin, fname) if err != nil { return nil, err @@ -152,7 +152,7 @@ func ParseConfig(ctx context.Context, fin io.Reader, fname string, defaultDiffic } if b.PathRegex != nil { - c, err := NewPathChecker(*b.PathRegex) + c, err := NewPathChecker(*b.PathRegex, subrequestMode) if err != nil { validationErrs = append(validationErrs, fmt.Errorf("while processing rule %s path regex: %w", b.Name, err)) } else { diff --git a/lib/policy/policy_test.go b/lib/policy/policy_test.go index 7f14d3f9..ff38fb36 100644 --- a/lib/policy/policy_test.go +++ b/lib/policy/policy_test.go @@ -19,7 +19,7 @@ func TestDefaultPolicyMustParse(t *testing.T) { } defer fin.Close() - if _, err := ParseConfig(ctx, fin, "botPolicies.yaml", anubis.DefaultDifficulty, "info"); err != nil { + if _, err := ParseConfig(ctx, fin, "botPolicies.yaml", anubis.DefaultDifficulty, "info", false); err != nil { t.Fatalf("can't parse config: %v", err) } } @@ -41,7 +41,7 @@ func TestGoodConfigs(t *testing.T) { defer fin.Close() ctx := thothmock.WithMockThoth(t) - if _, err := ParseConfig(ctx, fin, fin.Name(), anubis.DefaultDifficulty, "info"); err != nil { + if _, err := ParseConfig(ctx, fin, fin.Name(), anubis.DefaultDifficulty, "info", false); err != nil { t.Fatal(err) } }) @@ -53,7 +53,7 @@ func TestGoodConfigs(t *testing.T) { } defer fin.Close() - if _, err := ParseConfig(t.Context(), fin, fin.Name(), anubis.DefaultDifficulty, "info"); err != nil { + if _, err := ParseConfig(t.Context(), fin, fin.Name(), anubis.DefaultDifficulty, "info", false); err != nil { t.Fatal(err) } }) @@ -77,7 +77,7 @@ func TestBadConfigs(t *testing.T) { } defer fin.Close() - if _, err := ParseConfig(ctx, fin, fin.Name(), anubis.DefaultDifficulty, "info"); err == nil { + if _, err := ParseConfig(ctx, fin, fin.Name(), anubis.DefaultDifficulty, "info", false); err == nil { t.Fatal(err) } else { t.Log(err)