Przeglądaj źródła

fix(oauth2): reject empty state when no flow is in progress

The `sess.OAuth2State()` function returns "" when no flow has been initiated,
thus making `subtle.ConstantTimeCompare("", "")` return 1, making a callback
with state= passes this check.

This isn't an exploitable vulnerability, as PKCE is enforced for both Google
and OIDC (authorization.go:45-49, google.go:57-60) and the missing
code_verifier will fail at the IdP token exchange.

This was found as I was digging into Forgejo OAuth2's implementation, and
wondered how miniflux was faring.
jvoisin 1 tydzień temu
rodzic
commit
fc68e33681
1 zmienionych plików z 5 dodań i 2 usunięć
  1. 5 2
      internal/ui/oauth2_callback.go

+ 5 - 2
internal/ui/oauth2_callback.go

@@ -34,8 +34,11 @@ func (h *handler) oauth2Callback(w http.ResponseWriter, r *http.Request) {
 	sess := request.WebSession(r)
 
 	state := request.QueryStringParam(r, "state", "")
-	if subtle.ConstantTimeCompare([]byte(state), []byte(sess.OAuth2State())) == 0 {
-		slog.Warn("Invalid OAuth2 state value received")
+	expectedState := sess.OAuth2State()
+	if expectedState == "" || subtle.ConstantTimeCompare([]byte(state), []byte(expectedState)) == 0 {
+		slog.Warn("Invalid OAuth2 state value received",
+			slog.String("provider", provider),
+		)
 		response.HTMLRedirect(w, r, h.routePath("/"))
 		return
 	}