Every form handler reads its inputs with r.FormValue, which merges the URL query into r.Form behind the request body. When the body does not carry a name, the query string supplies it. So a password or a TOTP code sent as a query parameter is not merely logged somewhere, it is accepted.
Verified
Running it, rather than reading the standard library's documentation:
POST /auth/pwd?password=QUERY-SECRET&otp=123456 (empty body)
r.FormValue("password") = "QUERY-SECRET"
r.FormValue("otp") = "123456"
r.Form = map[otp:[123456] password:[QUERY-SECRET]]
r.PostForm = map[]
With the name present in both, the body wins (r.Form becomes [BODY-SECRET QUERY-SECRET] and FormValue returns the first). It is only the fallback that is the problem, and the fallback is always there.
Where
Nine call sites outside tests read a credential this way:
$ grep -rn 'FormValue("password")\|FormValue("otp")\|FormValue("currentPassword")' --include=*.go src/
src/authserver/internal/handlers/handler_auth_otp.go:245
src/authserver/internal/handlers/handler_reset_password.go:173
src/authserver/internal/handlers/handler_auth_pwd.go:132
src/authserver/internal/handlers/accounthandlers/handler_account_register.go:66
src/adminconsole/internal/handlers/accounthandlers/handler_account_change_password.go:62
src/adminconsole/internal/handlers/accounthandlers/handler_account_otp.go:80
src/adminconsole/internal/handlers/accounthandlers/handler_account_otp.go:126
src/adminconsole/internal/handlers/adminsettingshandlers/handler_admin_settings_email.go:104
src/adminconsole/internal/handlers/adminuserhandlers/handler_admin_user_new.go:90
Why it matters
A credential in a request target reaches browser history, the Referer header, and the access log of every intermediary between the client and the server. Accepting one is what makes a mistaken integration, a copied curl command or a crafted link work at all, and a request that works is a request that gets repeated.
Suggested fix
Read the credential-bearing names with r.PostFormValue, which is the body only, and leave r.FormValue for the parameters that legitimately arrive either way (/auth/authorize genuinely accepts both GET and POST per OpenID Connect, and its own reads should stay as they are). At minimum: password, currentPassword, newPassword, otp.
Related
Every form handler reads its inputs with
r.FormValue, which merges the URL query intor.Formbehind the request body. When the body does not carry a name, the query string supplies it. So a password or a TOTP code sent as a query parameter is not merely logged somewhere, it is accepted.Verified
Running it, rather than reading the standard library's documentation:
With the name present in both, the body wins (
r.Formbecomes[BODY-SECRET QUERY-SECRET]andFormValuereturns the first). It is only the fallback that is the problem, and the fallback is always there.Where
Nine call sites outside tests read a credential this way:
Why it matters
A credential in a request target reaches browser history, the
Refererheader, and the access log of every intermediary between the client and the server. Accepting one is what makes a mistaken integration, a copiedcurlcommand or a crafted link work at all, and a request that works is a request that gets repeated.Suggested fix
Read the credential-bearing names with
r.PostFormValue, which is the body only, and leaver.FormValuefor the parameters that legitimately arrive either way (/auth/authorizegenuinely accepts both GET and POST per OpenID Connect, and its own reads should stay as they are). At minimum:password,currentPassword,newPassword,otp.Related
r.FormValue("email")in the rate limiter, but about normalising the key rather than about which half of the request it comes from.