mirror of
https://github.com/rclone/rclone.git
synced 2026-10-28 14:28:38 -04:00
Nick Craig-Wood
a53fdac5ec
The local web server which receives the OAuth callback during `rclone config` and `rclone authorize` checks that the `state` parameter matches the random value rclone generated when starting the flow. The pcloud backend opted out of this check with the `StateBlankOK` option in lib/oauthutil, which made the local server accept a callback with no state at all. The callback URL (http://127.0.0.1:53682/) is fixed and well known, so while a pcloud OAuth flow was in progress any web page open in the user's browser could complete it with a single cross-site request carrying an attacker's authorization code. This is a CSRF (CWE-352) on the auth callback. This commit removes the `StateBlankOK` option so that every callback must carry the correct state and adds a test that the local auth server rejects callbacks with a blank, missing or wrong state. The option was added in May 2020 (#4210) because pCloud briefly omitted the state parameter from its redirect. The callbacks recorded five weeks later when EU region support was added show both api.pcloud.com and eapi.pcloud.com returning `state=` again, so the exemption has been unnecessary since then. See: GHSA-hvpr-p4pv-4f46
43 lines
1.3 KiB
Go
43 lines
1.3 KiB
Go
package oauthutil
|
|
|
|
import (
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
)
|
|
|
|
// TestAuthServerHandleAuth checks that only a callback carrying the
|
|
// random state may complete the flow, since the callback is an
|
|
// unauthenticated request any web page in the user's browser can make.
|
|
func TestAuthServerHandleAuth(t *testing.T) {
|
|
for _, test := range []struct {
|
|
name string
|
|
query string
|
|
wantStatus int
|
|
wantOK bool
|
|
}{
|
|
{"CorrectState", "?code=CODE&state=correct-state", http.StatusOK, true},
|
|
{"WrongState", "?code=CODE&state=wrong-state", http.StatusBadRequest, false},
|
|
{"BlankState", "?code=CODE&state=", http.StatusBadRequest, false},
|
|
{"MissingState", "?code=CODE", http.StatusBadRequest, false},
|
|
{"MissingCode", "?state=correct-state", http.StatusBadRequest, false},
|
|
} {
|
|
t.Run(test.name, func(t *testing.T) {
|
|
s := newAuthServer(&Options{}, "", "correct-state", "")
|
|
w := httptest.NewRecorder()
|
|
s.handleAuth(w, httptest.NewRequest("GET", "/"+test.query, nil))
|
|
assert.Equal(t, test.wantStatus, w.Code)
|
|
result := <-s.result
|
|
assert.Equal(t, test.wantOK, result.OK)
|
|
if test.wantOK {
|
|
assert.Equal(t, "CODE", result.Code)
|
|
assert.Equal(t, "CODE", result.Form.Get("code"))
|
|
} else {
|
|
assert.Equal(t, "", result.Code)
|
|
}
|
|
})
|
|
}
|
|
}
|