From a7a4666ddd560515bf3031bd3a82c769746cccc0 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 1d10384cc..55eb8ac0d 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 {