From 64ea56873d7aeaaf5c4c2acd407344df00300155 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Wed, 25 Mar 2026 11:16:34 +0100 Subject: [PATCH 1/5] fix: prevent nil pointer dereference in DPoP strip() for invalid htu URLs url.Parse returns (nil, error) for strings with invalid percent-encoded sequences (e.g. "%zz"). strip() ignored the error and dereferenced the nil pointer, causing a server panic. Two-layer fix in crypto/dpop/dpop.go: - Parse() now validates the htu claim is a parseable URL, so tokens with a malformed htu are rejected early with a clear error message. - strip() signature changed to (string, error) so any remaining parse failure is propagated through Match() rather than causing a nil deref. Adds a unit test (TestParseDPoP/invalid_htu_claim_URL) that covers the regression: a DPoP token with htu="%zz" is rejected at Parse() time. Co-Authored-By: Claude Sonnet 4.6 --- crypto/dpop/dpop.go | 29 ++++++++++++++++++++--------- crypto/dpop/dpop_test.go | 14 ++++++++++++++ 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/crypto/dpop/dpop.go b/crypto/dpop/dpop.go index 3f8ad8b932..14736f9baf 100644 --- a/crypto/dpop/dpop.go +++ b/crypto/dpop/dpop.go @@ -158,6 +158,8 @@ func Parse(s string) (*DPoP, error) { } if v, ok := token.Get(HTUKey); !ok || v == "" { return nil, fmt.Errorf("%w: missing htu claim", ErrInvalidDPoP) + } else if _, err := url.Parse(v.(string)); err != nil { + return nil, fmt.Errorf("%w: invalid htu claim: %w", ErrInvalidDPoP, err) } if v, ok := token.Get(HTMKey); !ok || v == "" { return nil, fmt.Errorf("%w: missing htm claim", ErrInvalidDPoP) @@ -220,8 +222,14 @@ func (t DPoP) Match(jkt string, method string, url string) (bool, error) { if method != t.HTM() { return false, fmt.Errorf("method mismatch, token: %s, given: %s", t.HTM(), method) } - urlLeft := strip(t.HTU()) - urlRight := strip(url) + urlLeft, err := strip(t.HTU()) + if err != nil { + return false, fmt.Errorf("invalid htu claim: %w", err) + } + urlRight, err := strip(url) + if err != nil { + return false, fmt.Errorf("invalid url: %w", err) + } if urlLeft != urlRight { return false, fmt.Errorf("url mismatch, token: %s, given: %s", urlLeft, urlRight) } @@ -229,13 +237,16 @@ func (t DPoP) Match(jkt string, method string, url string) (bool, error) { return true, nil } -func strip(raw string) string { - url, _ := url.Parse(raw) - url.Scheme = "https" - url.Host = strings.Split(url.Host, ":")[0] - url.RawQuery = "" - url.Fragment = "" - return url.String() +func strip(raw string) (string, error) { + u, err := url.Parse(raw) + if err != nil { + return "", err + } + u.Scheme = "https" + u.Host = strings.Split(u.Host, ":")[0] + u.RawQuery = "" + u.Fragment = "" + return u.String(), nil } func (t DPoP) MarshalJSON() ([]byte, error) { diff --git a/crypto/dpop/dpop_test.go b/crypto/dpop/dpop_test.go index 908fb8c7de..02524130e6 100644 --- a/crypto/dpop/dpop_test.go +++ b/crypto/dpop/dpop_test.go @@ -229,6 +229,20 @@ func TestParseDPoP(t *testing.T) { require.Error(t, err) assert.EqualError(t, err, "invalid DPoP token: jti claim too long") }) + t.Run("invalid htu claim URL", func(t *testing.T) { + // Regression test: url.Parse returns (nil, error) for invalid percent-encoded + // sequences. Without the fix, strip() dereferenced the nil pointer, panicking + // the server when Match() was called on a token with htu="%zz". + dpopToken := New(*request) + _ = dpopToken.Token.Set(HTUKey, "%zz") + dpopString, _ := dpopToken.Sign("kid", keyPair, alg) + + _, err := Parse(dpopString) + + require.Error(t, err) + assert.ErrorIs(t, err, ErrInvalidDPoP) + assert.Contains(t, err.Error(), "invalid htu claim") + }) } func TestDPoP_Match(t *testing.T) { From 749dc243c3006bd1598cf9b2b0a41d8b2c1ed90f Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Wed, 25 Mar 2026 13:23:30 +0100 Subject: [PATCH 2/5] fix: validate htu/htm claim types before type assertion in DPoP Parse() A non-string htu or htm claim (e.g. a JSON number or object) would pass the empty-string check but then panic on the v.(string) type assertion, reintroducing a nil/panic vector. Validate the type first so any non-string value is rejected with a clear error. Co-Authored-By: Claude Sonnet 4.6 --- crypto/dpop/dpop.go | 10 +++++++--- crypto/dpop/dpop_test.go | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/crypto/dpop/dpop.go b/crypto/dpop/dpop.go index 14736f9baf..67978cb3e4 100644 --- a/crypto/dpop/dpop.go +++ b/crypto/dpop/dpop.go @@ -156,12 +156,16 @@ func Parse(s string) (*DPoP, error) { if token.IssuedAt().IsZero() { return nil, fmt.Errorf("%w: missing iat claim", ErrInvalidDPoP) } - if v, ok := token.Get(HTUKey); !ok || v == "" { + if v, ok := token.Get(HTUKey); !ok { return nil, fmt.Errorf("%w: missing htu claim", ErrInvalidDPoP) - } else if _, err := url.Parse(v.(string)); err != nil { + } else if s, ok := v.(string); !ok || s == "" { + return nil, fmt.Errorf("%w: missing htu claim", ErrInvalidDPoP) + } else if _, err := url.Parse(s); err != nil { return nil, fmt.Errorf("%w: invalid htu claim: %w", ErrInvalidDPoP, err) } - if v, ok := token.Get(HTMKey); !ok || v == "" { + if v, ok := token.Get(HTMKey); !ok { + return nil, fmt.Errorf("%w: missing htm claim", ErrInvalidDPoP) + } else if s, ok := v.(string); !ok || s == "" { return nil, fmt.Errorf("%w: missing htm claim", ErrInvalidDPoP) } if token.JwtID() == "" { diff --git a/crypto/dpop/dpop_test.go b/crypto/dpop/dpop_test.go index 02524130e6..57038fa7bb 100644 --- a/crypto/dpop/dpop_test.go +++ b/crypto/dpop/dpop_test.go @@ -243,6 +243,26 @@ func TestParseDPoP(t *testing.T) { assert.ErrorIs(t, err, ErrInvalidDPoP) assert.Contains(t, err.Error(), "invalid htu claim") }) + t.Run("non-string htu claim", func(t *testing.T) { + dpopToken := New(*request) + _ = dpopToken.Token.Set(HTUKey, 42) + dpopString, _ := dpopToken.Sign("kid", keyPair, alg) + + _, err := Parse(dpopString) + + require.Error(t, err) + assert.EqualError(t, err, "invalid DPoP token: missing htu claim") + }) + t.Run("non-string htm claim", func(t *testing.T) { + dpopToken := New(*request) + _ = dpopToken.Token.Set(HTMKey, 42) + dpopString, _ := dpopToken.Sign("kid", keyPair, alg) + + _, err := Parse(dpopString) + + require.Error(t, err) + assert.EqualError(t, err, "invalid DPoP token: missing htm claim") + }) } func TestDPoP_Match(t *testing.T) { From ee1500108e734d5542ee9dedf9120be24dc2baaf Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Wed, 25 Mar 2026 13:25:32 +0100 Subject: [PATCH 3/5] fix: validate htu/htm claim types before type assertion in DPoP Parse() A non-string htu or htm claim (e.g. a JSON number or object) would pass the empty-string check but then panic on the v.(string) type assertion, reintroducing a nil/panic vector. Validate the type first so any non-string value is rejected with a clear error. Co-Authored-By: Claude Sonnet 4.6 --- crypto/dpop/dpop.go | 8 ++++++-- crypto/dpop/dpop_test.go | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/crypto/dpop/dpop.go b/crypto/dpop/dpop.go index 67978cb3e4..23144acd0d 100644 --- a/crypto/dpop/dpop.go +++ b/crypto/dpop/dpop.go @@ -158,14 +158,18 @@ func Parse(s string) (*DPoP, error) { } if v, ok := token.Get(HTUKey); !ok { return nil, fmt.Errorf("%w: missing htu claim", ErrInvalidDPoP) - } else if s, ok := v.(string); !ok || s == "" { + } else if s, ok := v.(string); !ok { + return nil, fmt.Errorf("%w: invalid htu claim", ErrInvalidDPoP) + } else if s == "" { return nil, fmt.Errorf("%w: missing htu claim", ErrInvalidDPoP) } else if _, err := url.Parse(s); err != nil { return nil, fmt.Errorf("%w: invalid htu claim: %w", ErrInvalidDPoP, err) } if v, ok := token.Get(HTMKey); !ok { return nil, fmt.Errorf("%w: missing htm claim", ErrInvalidDPoP) - } else if s, ok := v.(string); !ok || s == "" { + } else if s, ok := v.(string); !ok { + return nil, fmt.Errorf("%w: invalid htm claim", ErrInvalidDPoP) + } else if s == "" { return nil, fmt.Errorf("%w: missing htm claim", ErrInvalidDPoP) } if token.JwtID() == "" { diff --git a/crypto/dpop/dpop_test.go b/crypto/dpop/dpop_test.go index 57038fa7bb..fb2030d1ed 100644 --- a/crypto/dpop/dpop_test.go +++ b/crypto/dpop/dpop_test.go @@ -251,7 +251,7 @@ func TestParseDPoP(t *testing.T) { _, err := Parse(dpopString) require.Error(t, err) - assert.EqualError(t, err, "invalid DPoP token: missing htu claim") + assert.EqualError(t, err, "invalid DPoP token: invalid htu claim") }) t.Run("non-string htm claim", func(t *testing.T) { dpopToken := New(*request) @@ -261,7 +261,7 @@ func TestParseDPoP(t *testing.T) { _, err := Parse(dpopString) require.Error(t, err) - assert.EqualError(t, err, "invalid DPoP token: missing htm claim") + assert.EqualError(t, err, "invalid DPoP token: invalid htm claim") }) } From d3aac13863dfd742de47e3053e795826d5433864 Mon Sep 17 00:00:00 2001 From: reinkrul Date: Wed, 25 Mar 2026 13:26:04 +0100 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- crypto/dpop/dpop.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crypto/dpop/dpop.go b/crypto/dpop/dpop.go index 23144acd0d..c582599fe2 100644 --- a/crypto/dpop/dpop.go +++ b/crypto/dpop/dpop.go @@ -251,7 +251,9 @@ func strip(raw string) (string, error) { return "", err } u.Scheme = "https" - u.Host = strings.Split(u.Host, ":")[0] + if host := u.Hostname(); host != "" { + u.Host = host + } u.RawQuery = "" u.Fragment = "" return u.String(), nil From 897a2624a6ff74dee67b5423333073ad0e047cc7 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 14:53:33 +0100 Subject: [PATCH 5/5] refactor(dpop): eliminate variable shadowing in Parse() HTU/HTM claim validation (#4117) * Initial plan * refactor: rename shadowing s variables to htu/htm in Parse() Co-authored-by: reinkrul <1481228+reinkrul@users.noreply.github.com> Agent-Logs-Url: https://github.com/nuts-foundation/nuts-node/sessions/a3558c07-9536-44cd-b10a-74d497ac71f4 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: reinkrul <1481228+reinkrul@users.noreply.github.com> --- crypto/dpop/dpop.go | 11 +++++------ go.sum | 2 -- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/crypto/dpop/dpop.go b/crypto/dpop/dpop.go index c582599fe2..5e6e585814 100644 --- a/crypto/dpop/dpop.go +++ b/crypto/dpop/dpop.go @@ -30,7 +30,6 @@ import ( "net/http" "net/url" "slices" - "strings" "time" "github.com/lestrrat-go/jwx/v2/jwa" @@ -158,18 +157,18 @@ func Parse(s string) (*DPoP, error) { } if v, ok := token.Get(HTUKey); !ok { return nil, fmt.Errorf("%w: missing htu claim", ErrInvalidDPoP) - } else if s, ok := v.(string); !ok { + } else if htu, ok := v.(string); !ok { return nil, fmt.Errorf("%w: invalid htu claim", ErrInvalidDPoP) - } else if s == "" { + } else if htu == "" { return nil, fmt.Errorf("%w: missing htu claim", ErrInvalidDPoP) - } else if _, err := url.Parse(s); err != nil { + } else if _, err := url.Parse(htu); err != nil { return nil, fmt.Errorf("%w: invalid htu claim: %w", ErrInvalidDPoP, err) } if v, ok := token.Get(HTMKey); !ok { return nil, fmt.Errorf("%w: missing htm claim", ErrInvalidDPoP) - } else if s, ok := v.(string); !ok { + } else if htm, ok := v.(string); !ok { return nil, fmt.Errorf("%w: invalid htm claim", ErrInvalidDPoP) - } else if s == "" { + } else if htm == "" { return nil, fmt.Errorf("%w: missing htm claim", ErrInvalidDPoP) } if token.JwtID() == "" { diff --git a/go.sum b/go.sum index 2ebaaae442..80d16f4f43 100644 --- a/go.sum +++ b/go.sum @@ -398,8 +398,6 @@ github.com/nuts-foundation/crypto-ecies v0.0.0-20211207143025-5b84f9efce2b h1:80 github.com/nuts-foundation/crypto-ecies v0.0.0-20211207143025-5b84f9efce2b/go.mod h1:6YUioYirD6/8IahZkoS4Ypc8xbeJW76Xdk1QKcziNTM= github.com/nuts-foundation/go-did v0.18.0 h1:IB0X8PrzDulpR1zAgDpaHfwoSjJpIhx5u1Tg8I2nnb8= github.com/nuts-foundation/go-did v0.18.0/go.mod h1:4od1gAmCi9HjHTQGEvHC8pLeuXdXACxidAcdA52YScc= -github.com/nuts-foundation/go-leia/v4 v4.2.0 h1:o/bgYVCyTgsfgtaKmlrcUaJ2z4NwetERC98SUWwYajM= -github.com/nuts-foundation/go-leia/v4 v4.2.0/go.mod h1:Gw6bXqJLOAmHSiXJJYbVoj+Mowp/PoBRywO0ZPsVzA0= github.com/nuts-foundation/go-leia/v4 v4.3.0 h1:R0qGISIeg2q/PCQTC9cuoBtA6cFu4WBV2DbmSOWKZyM= github.com/nuts-foundation/go-leia/v4 v4.3.0/go.mod h1:Gw6bXqJLOAmHSiXJJYbVoj+Mowp/PoBRywO0ZPsVzA0= github.com/nuts-foundation/go-stoabs v1.11.0 h1:q18jVruPdFcVhodDrnKuhq/24i0pUC/YXgzJS0glKUU=