Skip to content

Commit b64679e

Browse files
committed
feat: Add fully featured wildcard detection on redirect uris
Signed-off-by: Sebastian Gaviria Tangarife <sgt.911@outlook.com>
1 parent eff4f06 commit b64679e

4 files changed

Lines changed: 125 additions & 9 deletions

File tree

cmd/dex/serve.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"os"
1515
"os/signal"
1616
"path/filepath"
17-
"regexp"
1817
"runtime"
1918
"strings"
2019
"sync/atomic"
@@ -38,6 +37,7 @@ import (
3837

3938
"github.com/dexidp/dex/api/v2"
4039
"github.com/dexidp/dex/pkg/featureflags"
40+
dexRegexp "github.com/dexidp/dex/pkg/regexp"
4141
"github.com/dexidp/dex/server"
4242
"github.com/dexidp/dex/server/apiserver"
4343
"github.com/dexidp/dex/server/authflow"
@@ -244,15 +244,17 @@ func runServe(options serveOptions) error {
244244
c.StaticClients[i].Secret = os.Getenv(client.SecretEnv)
245245
}
246246
if client.InsecureAllowRegexpRedirectURIs {
247+
logger.Warn("using flag InsecureAllowRegexpRedirectURIs", "client", client.ID)
247248
for _, uri := range client.RedirectURIs {
248-
if !client.InsecureAllowWildcardRedirectURIs && strings.Contains(uri, ".*") {
249-
return fmt.Errorf("invalid config: InsecureAllowWildcardRedirectURIs is required when using \".*\"")
249+
hasArbitraryWildcards, err := dexRegexp.HasArbitraryWildcard(uri)
250+
if err != nil {
251+
return fmt.Errorf("invalid config: RedirectURI %q is not a valid regexp expression: %w", uri, err)
250252
}
251253

252-
_, err := regexp.Compile(uri)
253-
if err != nil {
254-
return fmt.Errorf("invalid config: RedirectURI %q is not a valid regexp expression", uri)
254+
if !client.InsecureAllowWildcardRedirectURIs && hasArbitraryWildcards {
255+
return fmt.Errorf("invalid config: InsecureAllowWildcardRedirectURIs is required when using any unrestricted wildcard")
255256
}
257+
256258
}
257259
}
258260
logger.Info("config static client", "client_name", client.Name)

pkg/regexp/regexp.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package regexp
2+
3+
import (
4+
"regexp/syntax"
5+
"slices"
6+
)
7+
8+
// isNegatedOrBroadClass determines whether a character class (e.g., [0-9], [^/], [a-z])
9+
// allows a character range wide enough to act as an unrestricted wildcard.
10+
//
11+
// In Go's regex AST (regexp/syntax), character classes (syntax.OpCharClass) are represented
12+
// in the 'Rune' slice as a sequence of inclusive range pairs: [start1, end1, start2, end2, ...].
13+
//
14+
// For example:
15+
// - [0-9] is encoded as: ['0', '9'] -> 10 total characters
16+
// - [a-zA-Z] is encoded as: ['A', 'Z', 'a', 'z'] -> 52 total characters
17+
// - [^/] (negated class) expands into two vast ranges:
18+
// [UnicodeMin, '/'-1, '/'+1, UnicodeMax] -> >1,100,000 characters
19+
//
20+
// If the total number of allowed characters exceeds the threshold (100), the class is
21+
// considered either a negated class (like [^/]) or overly permissive, and thus classified
22+
// as an unrestricted wildcard.
23+
func isNegatedOrBroadClass(runes []rune) bool {
24+
var totalChars int32
25+
26+
// Rune ranges are always stored in start/end pairs at even/odd indices:
27+
// runes[i] = range start
28+
// runes[i+1] = range end
29+
for i := 0; i < len(runes); i += 2 {
30+
totalChars += (runes[i+1] - runes[i] + 1)
31+
}
32+
33+
// Safety threshold: Allows common URL character sets like [a-zA-Z0-9_-] (~65 chars),
34+
// while flagging negated classes or broad ranges that could match arbitrary subdomains.
35+
return totalChars > 100
36+
}
37+
38+
func inspectForWildcardSegments(node *syntax.Regexp) bool {
39+
// Inspect for child elements first
40+
// Any child element has an unrestricted wildcard
41+
if slices.ContainsFunc(node.Sub, inspectForWildcardSegments) {
42+
return true
43+
}
44+
45+
switch node.Op {
46+
// Unrestricter char wildcard (AKA. the dot)
47+
case syntax.OpAnyChar, syntax.OpAnyCharNotNL:
48+
return true
49+
50+
// Check for negated class chars with a wide allowed chars, marked as unrestricted wildcard.
51+
// Ex: [^/], [^#], many others...
52+
case syntax.OpCharClass:
53+
if isNegatedOrBroadClass(node.Rune) {
54+
return true
55+
}
56+
}
57+
58+
return false
59+
}
60+
61+
// HasArbitraryWildcard parses the regex pattern to check if it contains unrestricted wildcards.
62+
//
63+
// An unrestricted wildcard includes constructs like '.', '.*', '.+', or broad/negated character
64+
// classes like '[^/]+'.
65+
//
66+
// NOTE: Unescaped literal dots in domain names (e.g., "example.com" instead of "example\.com")
67+
// are parsed as syntax.OpAnyCharNotNL and will be flagged as wildcards. This prevents subtle
68+
// open-redirect vulnerabilities where "pr-123.example.com" could match "pr-123xexample.com"
69+
func HasArbitraryWildcard(pattern string) (bool, error) {
70+
ast, err := syntax.Parse(pattern, syntax.Perl)
71+
if err != nil {
72+
return false, err
73+
}
74+
75+
return inspectForWildcardSegments(ast), nil
76+
}

server/authflow/request.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
"github.com/coreos/go-oidc/v3/oidc"
1616

17+
dexRegexp "github.com/dexidp/dex/pkg/regexp"
1718
conns "github.com/dexidp/dex/server/connectors"
1819
"github.com/dexidp/dex/server/oauth2"
1920
"github.com/dexidp/dex/server/signer"
@@ -139,7 +140,9 @@ func isHostLocal(host string) bool {
139140

140141
func validateRegexpRedirectURI(redirectURIs []string, redirectURI string, allowWildcard bool) bool {
141142
for _, uri := range redirectURIs {
142-
if !allowWildcard && strings.Contains(uri, ".*") {
143+
// NOTE: This is also validated during server startup, but is safely skipped also during validation.
144+
hasArbitraryWildcards, err := dexRegexp.HasArbitraryWildcard(uri)
145+
if err != nil || (!allowWildcard && hasArbitraryWildcards) {
143146
continue
144147
}
145148

@@ -148,7 +151,7 @@ func validateRegexpRedirectURI(redirectURIs []string, redirectURI string, allowW
148151
continue
149152
}
150153

151-
if rgx.Match([]byte(redirectURI)) {
154+
if rgx.MatchString(redirectURI) {
152155
return true
153156
}
154157
}

server/authflow/request_test.go

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func TestParseAuthorizationRequest(t *testing.T) {
175175
{
176176
ID: "bar",
177177
InsecureAllowRegexpRedirectURIs: true,
178-
RedirectURIs: []string{`https://pr-(\d+).example.com`},
178+
RedirectURIs: []string{`https://pr-(\d+)\.example\.com`},
179179
},
180180
},
181181
supportedResponseTypes: []string{"code", "id_token", "token"},
@@ -258,6 +258,41 @@ func TestParseAuthorizationRequest(t *testing.T) {
258258
},
259259
expectedError: &displayedAuthErr{Status: http.StatusBadRequest},
260260
},
261+
{
262+
name: "wildcard url without flag alternate",
263+
clients: []storage.Client{
264+
{
265+
ID: "bar",
266+
InsecureAllowRegexpRedirectURIs: true,
267+
RedirectURIs: []string{`https?://[^/]`},
268+
},
269+
},
270+
supportedResponseTypes: []string{"code", "id_token", "token"},
271+
queryParams: map[string]string{
272+
"client_id": "bar",
273+
"redirect_uri": "https://example.com",
274+
"response_type": "code",
275+
"scope": "openid email profile",
276+
},
277+
expectedError: &displayedAuthErr{Status: http.StatusBadRequest},
278+
},
279+
{
280+
name: "wildcard url",
281+
clients: []storage.Client{
282+
{
283+
ID: "bar",
284+
InsecureAllowRegexpRedirectURIs: true,
285+
RedirectURIs: []string{`http://127\.0\.0\.1:(\d+)`},
286+
},
287+
},
288+
supportedResponseTypes: []string{"code", "id_token", "token"},
289+
queryParams: map[string]string{
290+
"client_id": "bar",
291+
"redirect_uri": "http://127.0.0.1:12345",
292+
"response_type": "code",
293+
"scope": "openid email profile",
294+
},
295+
},
261296
{
262297
name: "choose second connector_id",
263298
clients: []storage.Client{

0 commit comments

Comments
 (0)