mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-10-15 04:18:03 +00:00
fix(settings): tolerate whitespace in header template delimiters (#148)
validateTemplateSecure matched safe functions with literal strings.Contains
("{{get "), so a space after {{ ("{{ get") failed the whitelist and
Config.Validate() rejected the config, surfacing as Traefik's opaque
"invalid handler type: <nil>". Go's text/template treats {{ get and {{get
identically, so the validator was stricter than the engine.
Normalize inner-delimiter whitespace before matching. This also closes a
security gap where a leading space let dangerous patterns ("{{ range",
"{{ call") evade detection.
Adds unit + regression coverage.
This commit is contained in:
@@ -750,6 +750,48 @@ func TestValidateTemplateSecure_CoverageBoost(t *testing.T) {
|
|||||||
template: "{{.Unknown}}",
|
template: "{{.Unknown}}",
|
||||||
shouldError: true,
|
shouldError: true,
|
||||||
},
|
},
|
||||||
|
// Issue #148: whitespace after {{ and before }} must be tolerated
|
||||||
|
{
|
||||||
|
name: "get with leading space (issue #148)",
|
||||||
|
template: "{{ get .Claims \"email\" }}",
|
||||||
|
shouldError: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "get with multiple leading spaces",
|
||||||
|
template: "{{ get .Claims \"email\"}}",
|
||||||
|
shouldError: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "default with spaces",
|
||||||
|
template: "{{ default \"unknown\" .Claims.email }}",
|
||||||
|
shouldError: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "claims with spaces",
|
||||||
|
template: "{{ .Claims.email }}",
|
||||||
|
shouldError: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "access token with spaces",
|
||||||
|
template: "{{ .AccessToken }}",
|
||||||
|
shouldError: false,
|
||||||
|
},
|
||||||
|
// Issue #148: space must NOT let dangerous patterns bypass detection
|
||||||
|
{
|
||||||
|
name: "range with leading space stays blocked",
|
||||||
|
template: "{{ range .Items }}{{.}}{{end}}",
|
||||||
|
shouldError: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "call with leading space stays blocked",
|
||||||
|
template: "{{ call .Func }}",
|
||||||
|
shouldError: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "disallowed claim with spaces stays blocked",
|
||||||
|
template: "{{ .Claims.password }}",
|
||||||
|
shouldError: true,
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
|
|||||||
@@ -18,6 +18,50 @@ func TestIssueRegressions(t *testing.T) {
|
|||||||
t.Run("Issue60_Missing_Claim_Fields", testIssue60MissingClaimFields)
|
t.Run("Issue60_Missing_Claim_Fields", testIssue60MissingClaimFields)
|
||||||
t.Run("Issue60_Safe_Template_Functions", testIssue60SafeTemplateFunctions)
|
t.Run("Issue60_Safe_Template_Functions", testIssue60SafeTemplateFunctions)
|
||||||
t.Run("Issue60_Double_Processing_Concern", testIssue60DoubleProcessing)
|
t.Run("Issue60_Double_Processing_Concern", testIssue60DoubleProcessing)
|
||||||
|
t.Run("Issue148_Template_Whitespace_Tolerance", testIssue148TemplateWhitespace)
|
||||||
|
}
|
||||||
|
|
||||||
|
// testIssue148TemplateWhitespace tests that header template validation tolerates
|
||||||
|
// whitespace inside Go template delimiters (e.g. "{{ get" with a leading space),
|
||||||
|
// which Go's text/template treats identically to "{{get". v1.0.26 rejected the
|
||||||
|
// spaced form, causing "invalid handler type: <nil>" at startup. See issue #148.
|
||||||
|
func testIssue148TemplateWhitespace(t *testing.T) {
|
||||||
|
baseConfig := func() *traefikoidc.Config {
|
||||||
|
c := traefikoidc.CreateConfig()
|
||||||
|
c.ProviderURL = "https://example.com"
|
||||||
|
c.ClientID = "test-client"
|
||||||
|
c.ClientSecret = "test-secret"
|
||||||
|
c.CallbackURL = "/callback"
|
||||||
|
c.SessionEncryptionKey = "test-encryption-key-32-characters"
|
||||||
|
return c
|
||||||
|
}
|
||||||
|
|
||||||
|
// Spaced delimiters must validate identically to the compact form.
|
||||||
|
validSpaced := []traefikoidc.TemplatedHeader{
|
||||||
|
{Name: "X-Get-Spaced", Value: "{{ get .Claims \"email\" }}"},
|
||||||
|
{Name: "X-Get-MultiSpace", Value: "{{ get .Claims \"email\"}}"},
|
||||||
|
{Name: "X-Default-Spaced", Value: "{{ default \"unknown\" .Claims.department }}"},
|
||||||
|
{Name: "X-Claims-Spaced", Value: "{{ .Claims.email }}"},
|
||||||
|
{Name: "X-AccessToken-Spaced", Value: "{{ .AccessToken }}"},
|
||||||
|
}
|
||||||
|
for _, header := range validSpaced {
|
||||||
|
config := baseConfig()
|
||||||
|
config.Headers = []traefikoidc.TemplatedHeader{header}
|
||||||
|
assert.NoError(t, config.Validate(),
|
||||||
|
"Spaced template should validate: %s", header.Value)
|
||||||
|
}
|
||||||
|
|
||||||
|
// A leading space must NOT smuggle a dangerous pattern past validation.
|
||||||
|
dangerousSpaced := []traefikoidc.TemplatedHeader{
|
||||||
|
{Name: "X-Bad-Range", Value: "{{ range .Items }}{{.}}{{end}}"},
|
||||||
|
{Name: "X-Bad-Call", Value: "{{ call .Func }}"},
|
||||||
|
}
|
||||||
|
for _, header := range dangerousSpaced {
|
||||||
|
config := baseConfig()
|
||||||
|
config.Headers = []traefikoidc.TemplatedHeader{header}
|
||||||
|
assert.Error(t, config.Validate(),
|
||||||
|
"Spaced dangerous template must still be rejected: %s", header.Value)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// testIssue53CSRFRegression tests the specific issue reported in GitHub issue #53
|
// testIssue53CSRFRegression tests the specific issue reported in GitHub issue #53
|
||||||
|
|||||||
+28
@@ -9,10 +9,32 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
"net/url"
|
"net/url"
|
||||||
"os"
|
"os"
|
||||||
|
"regexp"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// templateDelimWhitespace matches whitespace immediately inside Go template
|
||||||
|
// delimiters: after "{{" or before "}}". Go's text/template treats "{{ get",
|
||||||
|
// "{{ get" and "{{get" identically, so the security validator must collapse
|
||||||
|
// this whitespace before pattern-matching — otherwise a leading space both
|
||||||
|
// rejects valid templates (issue #148) and lets dangerous patterns such as
|
||||||
|
// "{{ range" slip past detection.
|
||||||
|
var templateDelimWhitespace = regexp.MustCompile(`\{\{[ \t]+|[ \t]+\}\}`)
|
||||||
|
|
||||||
|
// normalizeTemplateDelimiters collapses inner-delimiter whitespace so that
|
||||||
|
// "{{ get .Claims \"x\" }}" becomes "{{get .Claims \"x\"}}" for matching.
|
||||||
|
// Trim markers ("{{-", "-}}") are left intact: the dash is not whitespace, so
|
||||||
|
// they remain detectable as dangerous patterns.
|
||||||
|
func normalizeTemplateDelimiters(templateStr string) string {
|
||||||
|
return templateDelimWhitespace.ReplaceAllStringFunc(templateStr, func(m string) string {
|
||||||
|
if strings.HasPrefix(m, "{{") {
|
||||||
|
return "{{"
|
||||||
|
}
|
||||||
|
return "}}"
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
// TemplatedHeader represents a custom HTTP header with a templated value.
|
// TemplatedHeader represents a custom HTTP header with a templated value.
|
||||||
// The value can contain template expressions that will be evaluated for each
|
// The value can contain template expressions that will be evaluated for each
|
||||||
// authenticated request, such as {{.claims.email}} or {{.accessToken}}.
|
// authenticated request, such as {{.claims.email}} or {{.accessToken}}.
|
||||||
@@ -569,6 +591,12 @@ func (c *Config) Validate() error {
|
|||||||
// It checks for dangerous template patterns that could lead to code execution or data leaks
|
// It checks for dangerous template patterns that could lead to code execution or data leaks
|
||||||
// while allowing safe custom functions for field access and default values.
|
// while allowing safe custom functions for field access and default values.
|
||||||
func validateTemplateSecure(templateStr string) error {
|
func validateTemplateSecure(templateStr string) error {
|
||||||
|
// Normalize inner-delimiter whitespace up front so that "{{ get .Claims }}"
|
||||||
|
// is validated identically to "{{get .Claims}}" (issue #148). All matching
|
||||||
|
// below operates on the normalized form, which also prevents a stray space
|
||||||
|
// from smuggling a dangerous pattern (e.g. "{{ range") past detection.
|
||||||
|
templateStr = normalizeTemplateDelimiters(templateStr)
|
||||||
|
|
||||||
// Allow our specific safe custom functions
|
// Allow our specific safe custom functions
|
||||||
// These are added specifically to handle missing fields safely (issue #60)
|
// These are added specifically to handle missing fields safely (issue #60)
|
||||||
safeCustomFunctions := []string{
|
safeCustomFunctions := []string{
|
||||||
|
|||||||
Reference in New Issue
Block a user