Skip to content

Commit 0e8802c

Browse files
lestrratfrestr
andauthored
Merge pull request from GHSA-pvcr-v8j8-j5q3
* Add tests for empty protected headers * check for sig.protected == nil * Add one more case for missing protected headers in compact form * Update Changes * JWS: Check for sig.protected == nil on non-flattened input --------- Co-authored-by: Fredrik Strupe <fredrik@strupe.net>
1 parent ea70886 commit 0e8802c

File tree

3 files changed

+72
-0
lines changed

3 files changed

+72
-0
lines changed

Diff for: Changes

+10
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@ v2.0.19 UNRELEASED
1010
was caused by actual verification step or something else, for example, while fetching
1111
a key from datasource
1212

13+
[Security Fixes]
14+
* [jws] JWS messages formated in full JSON format (i.e. not the compact format, which
15+
consists of three base64 strings concatenated with a '.') with missing "protected"
16+
headers could cause a panic, thereby introducing a possiblity of a DoS.
17+
18+
This has been fixed so that the `jws.Parse` function succeeds in parsing a JWS message
19+
lacking a protected header. Calling `jws.Verify` on this same JWS message will result
20+
in a failed verification attempt. Note that this behavior will differ slightly when
21+
parsing JWS messages in compact form, which result in an error.
22+
1323
v2.0.18 03 Dec 2023
1424
[Security Fixes]
1525
* [jwe] A large number in p2c parameter for PBKDF2 based encryptions could cause a DoS attack,

Diff for: jws/jws_test.go

+52
Original file line numberDiff line numberDiff line change
@@ -1835,3 +1835,55 @@ func TestValidateKey(t *testing.T) {
18351835
_, err = jws.Verify(signed, jws.WithKey(jwa.RS256, pubKey), jws.WithValidateKey(true))
18361836
require.NoError(t, err, `jws.Verify should succeed`)
18371837
}
1838+
1839+
func TestEmptyProtectedField(t *testing.T) {
1840+
// MEMO: this was the only test case from the original report
1841+
// This passes. It should produce an invalid JWS message, but
1842+
// that's not `jws.Parse`'s problem.
1843+
_, err := jws.Parse([]byte(`{"signature": ""}`))
1844+
require.NoError(t, err, `jws.Parse should fail`)
1845+
1846+
// Also test that non-flattened serialization passes.
1847+
_, err = jws.Parse([]byte(`{"signatures": [{}]}`))
1848+
require.NoError(t, err, `jws.Parse should fail`)
1849+
1850+
// MEMO: rest of the cases are present to be extra pedantic about it
1851+
1852+
privKey, err := jwxtest.GenerateRsaJwk()
1853+
require.NoError(t, err, `jwxtest.GenerateRsaJwk should succeed`)
1854+
1855+
// This fails. `jws.Parse` works, but the subsequent verification
1856+
// workflow fails to verify anything without the presense of a signature or
1857+
// a protected header.
1858+
_, err = jws.Verify([]byte(`{"signature": ""}`), jws.WithKey(jwa.RS256, privKey))
1859+
require.Error(t, err, `jws.Parse should fail`)
1860+
1861+
// Create a valid signatre.
1862+
signed, err := jws.Sign([]byte("Lorem Ipsum"), jws.WithKey(jwa.RS256, privKey))
1863+
require.NoError(t, err, `jws.Sign should succeed`)
1864+
1865+
_, payload, signature, err := jws.SplitCompact(signed)
1866+
require.NoError(t, err, `jws.SplitCompact should succeed`)
1867+
1868+
// This fails as well. we have a valid signature and a valid
1869+
// key to verify it, but no protected headers
1870+
_, err = jws.Verify(
1871+
[]byte(fmt.Sprintf(`{"signature": "%s"}`, signature)),
1872+
jws.WithKey(jwa.RS256, privKey),
1873+
)
1874+
require.Error(t, err, `jws.Verify should fail`)
1875+
1876+
// Test for cases when we have an incomplete compact form JWS
1877+
var buf bytes.Buffer
1878+
buf.WriteRune('.')
1879+
buf.Write(payload)
1880+
buf.WriteRune('.')
1881+
buf.Write(signature)
1882+
invalidMessage := buf.Bytes()
1883+
1884+
// This is an error because the format is simply wrong.
1885+
// Whereas in the other JSON-based JWS's case the lack of protected field
1886+
// is not a SYNTAX error, this one is, and therefore we barf.
1887+
_, err = jws.Parse(invalidMessage)
1888+
require.Error(t, err, `jws.Parse should fail`)
1889+
}

Diff for: jws/message.go

+10
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,11 @@ func (m *Message) UnmarshalJSON(buf []byte) error {
278278
}
279279
sig.SetDecodeCtx(nil)
280280

281+
if sig.protected == nil {
282+
// Instead of barfing on a nil protected header, use an empty header
283+
sig.protected = NewHeaders()
284+
}
285+
281286
if i == 0 {
282287
if !getB64Value(sig.protected) {
283288
b64 = false
@@ -313,6 +318,11 @@ func (m *Message) UnmarshalJSON(buf []byte) error {
313318
sig.protected = prt
314319
}
315320

321+
if sig.protected == nil {
322+
// Instead of barfing on a nil protected header, use an empty header
323+
sig.protected = NewHeaders()
324+
}
325+
316326
decoded, err := base64.DecodeString(*mup.Signature)
317327
if err != nil {
318328
return fmt.Errorf(`failed to base64 decode flattened signature: %w`, err)

0 commit comments

Comments
 (0)