Skip to content

Commit

Permalink
Ignore port when the redirect URI is a loopback address
Browse files Browse the repository at this point in the history 
When the registered redirect URI is a loopback address(IPv4 or IPv6)
the URI validation should ignore any port which is specified along with
the loopback address by the client. This is prescribed in
[rfc8252](https://www.rfc-editor.org/rfc/rfc8252.html#section-7.3) which
cover OAuth Authorization Code Grant Flow for native applications.
  • Loading branch information
@arjunrn
arjunrn committed Mar 1, 2022
1 parent e57c573 commit dc0f0cb
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 42 deletions.
32 changes: 25 additions & 7 deletions urivalidate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package osin
import (
"errors"
"fmt"
"net"
"net/url"
"strings"
)
Expand All @@ -18,7 +19,7 @@ func newUriValidationError(msg string, base string, redirect string) UriValidati
return UriValidationError(fmt.Sprintf("%s: %s / %s", msg, base, redirect))
}

// Parse urls, resolving uri references to base url
// ParseUrls parses URLs, resolving references to the base url
func ParseUrls(baseUrl, redirectUrl string) (retBaseUrl, retRedirectUrl *url.URL, err error) {
var base, redirect *url.URL
// parse base url
Expand All @@ -41,14 +42,31 @@ func ParseUrls(baseUrl, redirectUrl string) (retBaseUrl, retRedirectUrl *url.URL
return nil, nil, newUriValidationError("scheme mismatch", baseUrl, redirectUrl)
}

// Host must match
if redirect.Host != base.Host {
var (
redirectMatch bool
host string
)

// verify the hosts match
if redirect.Host == base.Host {
redirectMatch = true
host = base.Host
} else if baseIP := net.ParseIP(base.Host); baseIP != nil && baseIP.IsLoopback() && base.Scheme == "http" {
// if the registered redirect URI is lookback, then match the input redirect without the port.
if redirectIP := net.ParseIP(redirect.Hostname()); redirectIP != nil && redirectIP.IsLoopback() {
redirectMatch = true
host = redirect.Host
}
}

// Hosts must match
if !redirectMatch {
return nil, nil, newUriValidationError("host mismatch", baseUrl, redirectUrl)
}

// resolve references to base url
retBaseUrl = (&url.URL{Scheme: base.Scheme, Host: base.Host, Path: "/"}).ResolveReference(&url.URL{Path: base.Path})
retRedirectUrl = (&url.URL{Scheme: base.Scheme, Host: base.Host, Path: "/"}).ResolveReference(&url.URL{Path: redirect.Path, RawQuery: redirect.RawQuery})
retBaseUrl = (&url.URL{Scheme: base.Scheme, Host: host, Path: "/"}).ResolveReference(&url.URL{Path: base.Path})
retRedirectUrl = (&url.URL{Scheme: base.Scheme, Host: host, Path: "/"}).ResolveReference(&url.URL{Path: redirect.Path, RawQuery: redirect.RawQuery})
return
}

Expand Down Expand Up @@ -103,10 +121,10 @@ func ValidateUri(baseUri string, redirectUri string) (realRedirectUri string, er
return "", newUriValidationError("path prefix doesn't match", baseUri, redirectUri)
}

return redirect.String(),nil
return redirect.String(), nil
}

// Returns the first uri from an uri list
// FirstUri returns the first uri from a URI list
func FirstUri(baseUriList string, separator string) string {
if separator != "" {
slist := strings.Split(baseUriList, separator)
Expand Down
138 changes: 103 additions & 35 deletions urivalidate_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,131 +1,199 @@
package osin

import (
"fmt"
"testing"
)

func TestURIValidate(t *testing.T) {
valid := [][]string{
{
// Exact match
valid := []struct {
name string
clientRedirectURI string
inputRedirectURI string
normalized string
}{
{
"Exact match",
"http://localhost:14000/appauth",
"http://localhost:14000/appauth",
"http://localhost:14000/appauth",
},
{
// Trailing slash
"Trailing slash",
"http://www.google.com/myapp",
"http://www.google.com/myapp/",
"http://www.google.com/myapp/",
},
{
// Exact match with trailing slash
"Exact match with trailing slash",
"http://www.google.com/myapp/",
"http://www.google.com/myapp/",
"http://www.google.com/myapp/",
},
{
// Subpath
"Subpath",
"http://www.google.com/myapp",
"http://www.google.com/myapp/interface/implementation",
"http://www.google.com/myapp/interface/implementation",
},
{
// Subpath with trailing slash
"Subpath with trailing slash",
"http://www.google.com/myapp/",
"http://www.google.com/myapp/interface/implementation",
"http://www.google.com/myapp/interface/implementation",
},
{
// Subpath with things that are close to path traversals, but aren't
"Subpath with things that are close to path traversals, but aren't",
"http://www.google.com/myapp",
"http://www.google.com/myapp/.../..implementation../...",
"http://www.google.com/myapp/.../..implementation../...",
},
{
// If the allowed basepath contains path traversals, allow them?
"If the allowed basepath contains path traversals, allow them?",
"http://www.google.com/traversal/../allowed",
"http://www.google.com/traversal/../allowed/with/subpath",
"http://www.google.com/allowed/with/subpath",
},
{
// Backslashes
"Backslashes",
"https://mysafewebsite.com/secure/redirect",
"https://mysafewebsite.com/secure/redirect/\\../\\../\\../evil",
"https://mysafewebsite.com/secure/redirect/%5C../%5C../%5C../evil",
},
{
// Backslashes
"https://mysafewebsite.com/secure/redirect",
"https://mysafewebsite.com/secure/redirect/\\..\\../\\../evil",
"https://mysafewebsite.com/secure/redirect/%5C..%5C../%5C../evil",
},
{
// Query string must be kept
"Query string must be kept",
"http://www.google.com/myapp/redir",
"http://www.google.com/myapp/redir?a=1&b=2",
"http://www.google.com/myapp/redir?a=1&b=2",
},
{
"IPv4 loopback address",
"http://127.0.0.1/callback",
"http://127.0.0.1:8081/callback",
"http://127.0.0.1:8081/callback",
},
{
"Uncommon IPv4 loopback address",
"http://127.0.0.1/callback",
"http://127.0.0.8:8081/callback",
"http://127.0.0.8:8081/callback",
},
{
"IPv6 loopback address",
"http://127.0.0.1/callback",
"http://[::1]:8081/callback",
"http://[::1]:8081/callback",
},
{
"No port in IPv4 loopback address",
"http://127.0.0.1/callback",
"http://127.0.0.1/callback",
"http://127.0.0.1/callback",
},
{
"No port in IPv6 loopback address",
"http://127.0.0.1/callback",
"http://[0:0:0:0:0:0:0:1]/callback",
"http://[0:0:0:0:0:0:0:1]/callback",
},
}
for _, v := range valid {
if realRedirectUri, err := ValidateUri(v[0], v[1]); err != nil {
t.Errorf("Expected ValidateUri(%s, %s) to succeed, got %v", v[0], v[1], err)
} else if len(v) == 3 && realRedirectUri != v[2] {
t.Errorf("Expected ValidateUri(%s, %s) to return uri %s, got %s", v[0], v[1], v[2], realRedirectUri)
}
t.Run(fmt.Sprintf("valid/%s", v.name), func(t *testing.T) {
if realRedirectUri, err := ValidateUri(v.clientRedirectURI, v.inputRedirectURI); err != nil {
t.Errorf("Expected ValidateUri(%s, %s) to succeed, got %v", v.clientRedirectURI, v.inputRedirectURI, err)
} else if realRedirectUri != v.normalized {
t.Errorf("Expected ValidateUri(%s, %s) to return uri %s, got %s", v.clientRedirectURI, v.inputRedirectURI, v.normalized, realRedirectUri)
}
})
}

invalid := [][]string{
invalid := []struct {
name string
clientRedirectURI string
inputRedirectURI string
}{
{
// Doesn't satisfy base path
"Doesn't satisfy base path",
"http://localhost:14000/appauth",
"http://localhost:14000/app",
},
{
// Doesn't satisfy base path
"Doesn't satisfy base path",
"http://localhost:14000/app/",
"http://localhost:14000/app",
},
{
// Not a subpath of base path
"Not a subpath of base path",
"http://localhost:14000/appauth",
"http://localhost:14000/appauthmodifiedpath",
},
{
// Host mismatch
"Host mismatch",
"http://www.google.com/myapp",
"http://www2.google.com/myapp",
},
{
// Scheme mismatch
"Scheme mismatch",
"http://www.google.com/myapp",
"https://www.google.com/myapp",
},
{
// Path traversal
"Path traversal",
"http://www.google.com/myapp",
"http://www.google.com/myapp/..",
},
{
// Embedded path traversal
"Embedded path traversal",
"http://www.google.com/myapp",
"http://www.google.com/myapp/../test",
},
{
// Not a subpath
"Not a subpath",
"http://www.google.com/myapp",
"http://www.google.com/myapp../test",
},
{
// Backslashes
"Backslashes",
"https://mysafewebsite.com/secure/redirect",
"https://mysafewebsite.com/secure%2fredirect/../evil",
},
{
"Mismatching ports",
"https://example.com:8081/redirect",
"https://example.com:8080/redirect",
},
{
"Non loopback address",
"http://127.0.0.1/callback",
"http://localhost:8080/callback",
},
{
"Invalid input redirect URI",
"http://127.0.0.1/callback",
"http://127.0.0.1:abc/callback",
},
{
"Non http scheme in redirect URI",
"custom://127.0.0.1/callback",
"custom://127.0.0.1:8080/callback",
},
{
"Redirect URI is loopback, input is a domain name with port",
"http://127.0.0.1/callback",
"http://example.com:8081/callback",
},
{
"Redirect URI is loopback, input is a domain name without port",
"http://127.0.0.1/callback",
"http://example.com/callback",
},
}
for _, v := range invalid {
if _, err := ValidateUri(v[0], v[1]); err == nil {
t.Errorf("Expected ValidateUri(%s, %s) to fail", v[0], v[1])
}
t.Run(fmt.Sprintf("invalid/%s", v.name), func(t *testing.T) {
if _, err := ValidateUri(v.clientRedirectURI, v.inputRedirectURI); err == nil {
t.Errorf("Expected ValidateUri(%s, %s) to fail", v.clientRedirectURI, v.inputRedirectURI)
}
})
}
}

Expand Down

0 comments on commit dc0f0cb

Please sign in to comment.