From 349112df6b29cade0dc7b1d4d714a0391973c119 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Fri, 27 Sep 2019 14:59:56 +0100
Subject: [PATCH] oauthutil: fix security problem when running with two users
 on the same machine

Before this change two users could run `rclone config` for the same
backend on the same machine at the same time.

User A would get as far as starting the web server.  User B would then
fail to start the webserver, but it would open the browser on the
/auth URL which would redirect the user to the login.  This would then
cause user B to authenticate to user A's rclone.

This changes fixes the problem in two ways.

Firstly it passes the state to the /auth call before redirecting and
checks it there, erroring with a 403 error if it doesn't match.  This
would have fixed the problem on its own.

Secondly it delays the opening of the web browser until after the auth
webserver has started which prevents the user entering the credentials
if another auth server is running.

Fixes #3573
---
 lib/oauthutil/oauthutil.go | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/lib/oauthutil/oauthutil.go b/lib/oauthutil/oauthutil.go
index d0ed4f344..3d9e031a0 100644
--- a/lib/oauthutil/oauthutil.go
+++ b/lib/oauthutil/oauthutil.go
@@ -6,7 +6,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"html/template"
-	"log"
 	"net"
 	"net/http"
 	"strings"
@@ -445,9 +444,13 @@ func doConfig(id, name string, m configmap.Mapper, errorHandler func(*http.Reque
 	if useWebServer {
 		server.code = make(chan string, 1)
 		server.err = make(chan error, 1)
-		go server.Start()
+		err := server.Init()
+		if err != nil {
+			return errors.Wrap(err, "failed to start auth webserver")
+		}
+		go server.Serve()
 		defer server.Stop()
-		authURL = "http://" + bindAddress + "/auth"
+		authURL = "http://" + bindAddress + "/auth?state=" + state
 	}
 
 	// Generate a URL for the user to visit for authorization.
@@ -517,8 +520,8 @@ type AuthResponseData struct {
 	AuthError
 }
 
-// startWebServer runs an internal web server to receive config details
-func (s *authServer) Start() {
+// Init gets the internal web server ready to receive config details
+func (s *authServer) Init() error {
 	fs.Debugf(nil, "Starting auth server on %s", s.bindAddress)
 	mux := http.NewServeMux()
 	s.server = &http.Server{
@@ -531,6 +534,12 @@ func (s *authServer) Start() {
 		return
 	})
 	mux.HandleFunc("/auth", func(w http.ResponseWriter, req *http.Request) {
+		state := req.FormValue("state")
+		if state != s.state {
+			fs.Debugf(nil, "State did not match: want %q got %q", s.state, state)
+			http.Error(w, "State did not match - please try again", 403)
+			return
+		}
 		http.Redirect(w, req, s.authURL, http.StatusTemporaryRedirect)
 		return
 	})
@@ -585,12 +594,18 @@ func (s *authServer) Start() {
 	var err error
 	s.listener, err = net.Listen("tcp", s.bindAddress)
 	if err != nil {
-		log.Fatalf("Failed to start auth webserver: %v", err)
+		return err
 	}
-	err = s.server.Serve(s.listener)
+	return nil
+}
+
+// Serve the auth server, doesn't return
+func (s *authServer) Serve() {
+	err := s.server.Serve(s.listener)
 	fs.Debugf(nil, "Closed auth server with error: %v", err)
 }
 
+// Stop the auth server by closing its socket
 func (s *authServer) Stop() {
 	fs.Debugf(nil, "Closing auth server")
 	if s.code != nil {