diff --git a/CHANGELOG.md b/CHANGELOG.md index 236666af..f35a3b43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ ## Changes since v5.1.1 +- [#601](https://github.com/oauth2-proxy/oauth2-proxy/pull/601) Ensure decrypted user/email are valid UTF8 (@JoelSpeed) - [#560](https://github.com/oauth2-proxy/oauth2-proxy/pull/560) Fallback to UserInfo is User ID claim not present (@JoelSpeed) - [#598](https://github.com/oauth2-proxy/oauth2-proxy/pull/598) acr_values no longer sent to IdP when empty (@ScottGuymer) - [#548](https://github.com/oauth2-proxy/oauth2-proxy/pull/548) Separate logging options out of main options structure (@JoelSpeed) diff --git a/pkg/apis/sessions/session_state.go b/pkg/apis/sessions/session_state.go index 1bb8ff03..f2e6633e 100644 --- a/pkg/apis/sessions/session_state.go +++ b/pkg/apis/sessions/session_state.go @@ -2,8 +2,10 @@ package sessions import ( "encoding/json" + "errors" "fmt" "time" + "unicode/utf8" "github.com/oauth2-proxy/oauth2-proxy/pkg/encryption" ) @@ -106,6 +108,9 @@ func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) { if ss.Email != "" { decryptedEmail, errEmail := c.Decrypt(ss.Email) if errEmail == nil { + if !utf8.ValidString(decryptedEmail) { + return nil, errors.New("invalid value for decrypted email") + } ss.Email = decryptedEmail } } @@ -113,6 +118,9 @@ func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) { if ss.User != "" { decryptedUser, errUser := c.Decrypt(ss.User) if errUser == nil { + if !utf8.ValidString(decryptedUser) { + return nil, errors.New("invalid value for decrypted user") + } ss.User = decryptedUser } } diff --git a/pkg/apis/sessions/session_state_test.go b/pkg/apis/sessions/session_state_test.go index 529656fe..150e9c9d 100644 --- a/pkg/apis/sessions/session_state_test.go +++ b/pkg/apis/sessions/session_state_test.go @@ -49,15 +49,7 @@ func TestSessionStateSerialization(t *testing.T) { // ensure a different cipher can't decode properly (ie: it gets gibberish) ss, err = sessions.DecodeSessionState(encoded, c2) t.Logf("%#v", ss) - assert.Equal(t, nil, err) - assert.NotEqual(t, "user@domain.com", ss.User) - assert.NotEqual(t, s.Email, ss.Email) - assert.NotEqual(t, s.PreferredUsername, ss.PreferredUsername) - assert.Equal(t, s.CreatedAt.Unix(), ss.CreatedAt.Unix()) - assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix()) - assert.NotEqual(t, s.AccessToken, ss.AccessToken) - assert.NotEqual(t, s.IDToken, ss.IDToken) - assert.NotEqual(t, s.RefreshToken, ss.RefreshToken) + assert.NotEqual(t, nil, err) } func TestSessionStateSerializationWithUser(t *testing.T) { @@ -91,14 +83,7 @@ func TestSessionStateSerializationWithUser(t *testing.T) { // ensure a different cipher can't decode properly (ie: it gets gibberish) ss, err = sessions.DecodeSessionState(encoded, c2) t.Logf("%#v", ss) - assert.Equal(t, nil, err) - assert.NotEqual(t, s.User, ss.User) - assert.NotEqual(t, s.Email, ss.Email) - assert.NotEqual(t, s.PreferredUsername, ss.PreferredUsername) - assert.Equal(t, s.CreatedAt.Unix(), ss.CreatedAt.Unix()) - assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix()) - assert.NotEqual(t, s.AccessToken, ss.AccessToken) - assert.NotEqual(t, s.RefreshToken, ss.RefreshToken) + assert.NotEqual(t, nil, err) } func TestSessionStateSerializationNoCipher(t *testing.T) { @@ -278,6 +263,14 @@ func TestDecodeSessionState(t *testing.T) { Cipher: c, Error: true, }, + { + SessionState: sessions.SessionState{ + Email: "user@domain.com", + User: "YmFzZTY0LWVuY29kZWQtdXNlcgo=", // Base64 encoding of base64-encoded-user + }, + Error: true, + Cipher: c, + }, } for i, tc := range testCases {