rc: reject unauthorised and unknown calls before reading the request GHSA-f8p4-8g4j-q3m2 CVE-PENDING

The rc server decoded the whole POST body before looking up the method
and checking whether it needed authorisation, then echoed the decoded
input back in the 403 or 404 error. A client that was not allowed to
call a method could still make rclone read, decode and re-encode its
request.

The method lookup and authorisation check now happen before the body
is read, and these errors no longer echo the request input.

fs/rc/rcserver/rcserver.go

@@ -197,6 +197,18 @@ func (s *Server) handlePost(w http.ResponseWriter, r *http.Request, path string)
 	ctx := r.Context()
 	contentType := r.Header.Get("Content-Type")
 
+	// Find the call and check authorisation before reading the
+	// body so rejected requests can't make the server use memory
+	call := rc.Calls.Get(path)
+	if call == nil {
+		writeError(path, rc.Params{}, w, fmt.Errorf("couldn't find method %q", path), http.StatusNotFound)
+		return
+	}
+	if !s.noAuth && !call.NoAuth && !s.server.UsingAuth() {
+		writeError(path, rc.Params{}, w, fmt.Errorf("authentication must be set up on the rc server to use %q or the --rc-no-auth flag must be in use", path), http.StatusForbidden)
+		return
+	}
+
 	// Without auth anyone who can connect may POST, so bound the
 	// memory a request can make the server use
 	if !s.noAuth && !s.server.UsingAuth() {
@@ -264,19 +276,6 @@ func (s *Server) handlePost(w http.ResponseWriter, r *http.Request, path string)
 		}
 	}
 
-	// Find the call
-	call := rc.Calls.Get(path)
-	if call == nil {
-		writeError(path, in, w, fmt.Errorf("couldn't find method %q", path), http.StatusNotFound)
-		return
-	}
-
-	// Check to see if it requires authorisation
-	if !s.noAuth && !call.NoAuth && !s.server.UsingAuth() {
-		writeError(path, in, w, fmt.Errorf("authentication must be set up on the rc server to use %q or the --rc-no-auth flag must be in use", path), http.StatusForbidden)
-		return
-	}
-
 	inOrig := in.Copy()
 
 	if call.NeedsRequest {

fs/rc/rcserver/rcserver_test.go

@@ -922,13 +922,34 @@ func TestBodyLimitWithoutAuth(t *testing.T) {
 		Status:      http.StatusRequestEntityTooLarge,
 		Contains:    tooLarge,
 	}, {
+		// Rejected before the body is read so nothing is echoed
 		Name:        "authCall",
 		URL:         "rc/noopauth",
 		Method:      "POST",
 		Body:        bigBody,
 		ContentType: "application/json",
-		Status:      http.StatusRequestEntityTooLarge,
-		Contains:    tooLarge,
+		Status:      http.StatusForbidden,
+		Expected: `{
+	"error": "authentication must be set up on the rc server to use \"rc/noopauth\" or the --rc-no-auth flag must be in use",
+	"input": {},
+	"path": "rc/noopauth",
+	"status": 403
+}
+`,
+	}, {
+		Name:        "unknownCall",
+		URL:         "rc/potato",
+		Method:      "POST",
+		Body:        `{"potato":1}`,
+		ContentType: "application/json",
+		Status:      http.StatusNotFound,
+		Expected: `{
+	"error": "couldn't find method \"rc/potato\"",
+	"input": {},
+	"path": "rc/potato",
+	"status": 404
+}
+`,
 	}, {
 		Name:        "form",
 		URL:         "rc/noop",
@@ -992,9 +1013,7 @@ func TestJobAuthRequired(t *testing.T) {
 		Status:      http.StatusForbidden,
 		Expected: `{
 	"error": "authentication must be set up on the rc server to use \"job/status\" or the --rc-no-auth flag must be in use",
-	"input": {
-		"jobid": 1
-	},
+	"input": {},
 	"path": "job/status",
 	"status": 403
 }