rc: stop global.* connection string options changing config CVE-2026-49980
A connection string can carry global.* options which change rclone's process-wide configuration (e.g. global.http_proxy). This is undesirable for the rc interface which was designed to have multiple users or connections at once. The rc interface has the `_config` mechanism for setting request scoped global config. This blocks global.* options on all rc paths by marking the context as a remote control request at the rc boundaries. fs.NewFs then skips applying global.* to the process-wide config for a marked context. The marker is reapplied in fs.CopyConfig, which is the call rclone uses to detach context but keep config. global.* options still apply to the individual backend they are set on, exactly like override.* options; they just no longer leak into the rest of the process. Remotes created directly on the command line are unaffected as are remotes defined in the config file. See: GHSA-qw24-gh76-8rvv
This commit is contained in:
@@ -87,6 +87,17 @@ commands or read arbitrary local files.
|
|||||||
|
|
||||||
Default Off.
|
Default Off.
|
||||||
|
|
||||||
|
### global.* connection string options and the rc
|
||||||
|
|
||||||
|
Remotes instantiated by the rc do not let [connection
|
||||||
|
string](/docs/#connection-strings) `global.*` options change rclone's
|
||||||
|
process-wide configuration. Remotes created directly on the command
|
||||||
|
line or defined in the config file are unaffected.
|
||||||
|
|
||||||
|
A `global.*` option still takes effect for the individual backend it
|
||||||
|
is set on (exactly like an `override.*` option), it just does not leak
|
||||||
|
into the global config for the rest of the process.
|
||||||
|
|
||||||
### --rc-serve-no-modtime
|
### --rc-serve-no-modtime
|
||||||
|
|
||||||
Set this flag to skip reading the modification time (can speed things up).
|
Set this flag to skip reading the modification time (can speed things up).
|
||||||
|
|||||||
+5
-1
@@ -802,11 +802,15 @@ func GetConfig(ctx context.Context) *ConfigInfo {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// CopyConfig copies the global config (if any) from srcCtx into
|
// CopyConfig copies the global config (if any) from srcCtx into
|
||||||
// dstCtx returning the new context.
|
// dstCtx returning the new context. It also copies the rc request
|
||||||
|
// marker if present.
|
||||||
func CopyConfig(dstCtx, srcCtx context.Context) context.Context {
|
func CopyConfig(dstCtx, srcCtx context.Context) context.Context {
|
||||||
if srcCtx == nil {
|
if srcCtx == nil {
|
||||||
return dstCtx
|
return dstCtx
|
||||||
}
|
}
|
||||||
|
if IsRCRequest(srcCtx) {
|
||||||
|
dstCtx = WithRCRequest(dstCtx)
|
||||||
|
}
|
||||||
c := srcCtx.Value(configContextKey)
|
c := srcCtx.Value(configContextKey)
|
||||||
if c == nil {
|
if c == nil {
|
||||||
return dstCtx
|
return dstCtx
|
||||||
|
|||||||
@@ -29,3 +29,21 @@ func TestGetConfig(t *testing.T) {
|
|||||||
config2ctx := GetConfig(ctx2)
|
config2ctx := GetConfig(ctx2)
|
||||||
assert.Equal(t, config2, config2ctx)
|
assert.Equal(t, config2, config2ctx)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The rc request marker must survive CopyConfig, which is how rclone
|
||||||
|
// does detach context but keep config.
|
||||||
|
func TestRCRequestContext(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
assert.False(t, IsRCRequest(ctx))
|
||||||
|
|
||||||
|
rcCtx := WithRCRequest(ctx)
|
||||||
|
assert.True(t, IsRCRequest(rcCtx))
|
||||||
|
|
||||||
|
// CopyConfig carries the marker even when there is no config to copy
|
||||||
|
assert.True(t, IsRCRequest(CopyConfig(context.Background(), rcCtx)))
|
||||||
|
// and when there is
|
||||||
|
rcCtx, _ = AddConfig(rcCtx)
|
||||||
|
assert.True(t, IsRCRequest(CopyConfig(context.Background(), rcCtx)))
|
||||||
|
// An unmarked context stays unmarked
|
||||||
|
assert.False(t, IsRCRequest(CopyConfig(context.Background(), ctx)))
|
||||||
|
}
|
||||||
|
|||||||
+18
-2
@@ -25,6 +25,22 @@ var (
|
|||||||
overriddenConfig = make(map[string]string)
|
overriddenConfig = make(map[string]string)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// rcRequestKey marks a context as created by a remote control (rc) request.
|
||||||
|
type rcRequestKeyType struct{}
|
||||||
|
|
||||||
|
var rcRequestKey = rcRequestKeyType{}
|
||||||
|
|
||||||
|
// WithRCRequest marks ctx as created by a remote control (rc) request.
|
||||||
|
func WithRCRequest(ctx context.Context) context.Context {
|
||||||
|
return context.WithValue(ctx, rcRequestKey, true)
|
||||||
|
}
|
||||||
|
|
||||||
|
// IsRCRequest returns true if ctx was created by a remote control (rc) request.
|
||||||
|
func IsRCRequest(ctx context.Context) bool {
|
||||||
|
v, _ := ctx.Value(rcRequestKey).(bool)
|
||||||
|
return v
|
||||||
|
}
|
||||||
|
|
||||||
// NewFs makes a new Fs object from the path
|
// NewFs makes a new Fs object from the path
|
||||||
//
|
//
|
||||||
// The path is of the form remote:path
|
// The path is of the form remote:path
|
||||||
@@ -115,8 +131,8 @@ func addConfigToContext(ctx context.Context, configName string, config configmap
|
|||||||
return ctx, fmt.Errorf("failed to set override config variables %q: %w", overrideKeys, err)
|
return ctx, fmt.Errorf("failed to set override config variables %q: %w", overrideKeys, err)
|
||||||
}
|
}
|
||||||
Debugf(configName, "Set overridden config %q for backend startup", overrideKeys)
|
Debugf(configName, "Set overridden config %q for backend startup", overrideKeys)
|
||||||
// Set the global context only
|
// Set the global context unless this Fs is being created for an rc request
|
||||||
if len(globalConfig) != 0 {
|
if len(globalConfig) != 0 && !IsRCRequest(ctx) {
|
||||||
globalCI := GetConfig(context.Background())
|
globalCI := GetConfig(context.Background())
|
||||||
err = configstruct.Set(globalConfig, globalCI)
|
err = configstruct.Set(globalConfig, globalCI)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -53,3 +53,25 @@ func TestAddConfigToContext_GlobalOnly(t *testing.T) {
|
|||||||
ci := GetConfig(newCtx)
|
ci := GetConfig(newCtx)
|
||||||
assert.Equal(t, "potato2", ci.UserAgent)
|
assert.Equal(t, "potato2", ci.UserAgent)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// When the ctx is marked as a remote control (rc) request, a global.key must
|
||||||
|
// apply to the backend's own ctx but must NOT change the process-wide config.
|
||||||
|
func TestAddConfigToContext_GlobalFromRC(t *testing.T) {
|
||||||
|
global := configmap.Simple{
|
||||||
|
"global.user_agent": "potato3",
|
||||||
|
}
|
||||||
|
ctx := WithRCRequest(context.Background())
|
||||||
|
globalCI := GetConfig(ctx)
|
||||||
|
original := globalCI.UserAgent
|
||||||
|
defer func() {
|
||||||
|
globalCI.UserAgent = original
|
||||||
|
}()
|
||||||
|
newCtx, err := addConfigToContext(ctx, "unit-test", global)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.NotEqual(t, newCtx, ctx)
|
||||||
|
// The process-wide config must be untouched
|
||||||
|
assert.Equal(t, original, globalCI.UserAgent)
|
||||||
|
// but the backend's own ctx still gets the value
|
||||||
|
ci := GetConfig(newCtx)
|
||||||
|
assert.Equal(t, "potato3", ci.UserAgent)
|
||||||
|
}
|
||||||
|
|||||||
@@ -328,6 +328,9 @@ func (jobs *Jobs) NewJob(ctx context.Context, fn rc.Func, in rc.Params) (job *Jo
|
|||||||
// Add the job to the context
|
// Add the job to the context
|
||||||
ctx = context.WithValue(ctx, jobKey, job)
|
ctx = context.WithValue(ctx, jobKey, job)
|
||||||
|
|
||||||
|
// Mark the context as created from an rc request
|
||||||
|
ctx = fs.WithRCRequest(ctx)
|
||||||
|
|
||||||
if isAsync {
|
if isAsync {
|
||||||
go job.run(ctx, fn, in)
|
go job.run(ctx, fn, in)
|
||||||
out = make(rc.Params)
|
out = make(rc.Params)
|
||||||
|
|||||||
@@ -300,6 +300,37 @@ func TestExecuteJobWithConfig(t *testing.T) {
|
|||||||
assert.NotEqual(t, 42*fs.Mebi, ci.BufferSize)
|
assert.NotEqual(t, 42*fs.Mebi, ci.BufferSize)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// NewJob must mark the context as a remote control (rc) request, including for
|
||||||
|
// asynchronous jobs whose context is detached
|
||||||
|
func TestNewJobMarksRCRequest(t *testing.T) {
|
||||||
|
jobID.Store(0)
|
||||||
|
jobs := newJobs() // local instance so we don't pollute the global registry
|
||||||
|
|
||||||
|
// synchronous job
|
||||||
|
var syncMarked bool
|
||||||
|
_, _, err := jobs.NewJob(context.Background(), func(ctx context.Context, in rc.Params) (rc.Params, error) {
|
||||||
|
syncMarked = fs.IsRCRequest(ctx)
|
||||||
|
return nil, nil
|
||||||
|
}, rc.Params{})
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.True(t, syncMarked, "sync rc job context must be marked as an rc request")
|
||||||
|
|
||||||
|
// asynchronous job - the context is detached, so the marker must be set
|
||||||
|
// after that detachment to survive
|
||||||
|
done := make(chan bool, 1)
|
||||||
|
_, _, err = jobs.NewJob(context.Background(), func(ctx context.Context, in rc.Params) (rc.Params, error) {
|
||||||
|
done <- fs.IsRCRequest(ctx)
|
||||||
|
return nil, nil
|
||||||
|
}, rc.Params{"_async": true})
|
||||||
|
require.NoError(t, err)
|
||||||
|
select {
|
||||||
|
case asyncMarked := <-done:
|
||||||
|
assert.True(t, asyncMarked, "async rc job context must be marked as an rc request")
|
||||||
|
case <-time.After(5 * time.Second):
|
||||||
|
t.Fatal("async job did not run")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestExecuteJobWithFilter(t *testing.T) {
|
func TestExecuteJobWithFilter(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
called := false
|
called := false
|
||||||
|
|||||||
@@ -348,7 +348,7 @@ func (s *Server) serveRoot(w http.ResponseWriter, r *http.Request) {
|
|||||||
// Instantiating a backend from request-supplied configuration can execute
|
// Instantiating a backend from request-supplied configuration can execute
|
||||||
// commands during initialisation (e.g. webdav bearer_token_command, sftp ssh),
|
// commands during initialisation (e.g. webdav bearer_token_command, sftp ssh),
|
||||||
// read arbitrary local files, or mutate process-wide config via global.*
|
// read arbitrary local files, or mutate process-wide config via global.*
|
||||||
// options. See GHSA-qw24-gh76-8rvv.
|
// options so shouldn't be done without authentication.
|
||||||
//
|
//
|
||||||
// authenticated must be true if the request has been authenticated (HTTP auth
|
// authenticated must be true if the request has been authenticated (HTTP auth
|
||||||
// is configured on the server) or --rc-no-auth was passed to explicitly opt in
|
// is configured on the server) or --rc-no-auth was passed to explicitly opt in
|
||||||
@@ -391,7 +391,8 @@ func (s *Server) serveRemote(w http.ResponseWriter, r *http.Request, path string
|
|||||||
writeError(path, nil, w, err, http.StatusForbidden)
|
writeError(path, nil, w, err, http.StatusForbidden)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
f, err := cache.Get(s.ctx, fsName)
|
// Mark as an rc request e.g. so NewFs can reject global.* config
|
||||||
|
f, err := cache.Get(fs.WithRCRequest(s.ctx), fsName)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
writeError(path, nil, w, fmt.Errorf("failed to make Fs: %w", err), http.StatusInternalServerError)
|
writeError(path, nil, w, fmt.Errorf("failed to make Fs: %w", err), http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -480,6 +480,30 @@ func TestServeRemoteWithAuth(t *testing.T) {
|
|||||||
testServer(t, tests, &opt)
|
testServer(t, tests, &opt)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The serve path must mark backend creation as a remote control (rc) request
|
||||||
|
// so a request-supplied remote can't change process-wide config via global.*
|
||||||
|
// options.
|
||||||
|
func TestServeRemoteMarksRCRequest(t *testing.T) {
|
||||||
|
configfile.Install()
|
||||||
|
opt := newTestOpt()
|
||||||
|
opt.Serve = true
|
||||||
|
opt.NoAuth = true // allow inline remotes so we reach backend creation
|
||||||
|
opt.Template.Path = defaultTestTemplate
|
||||||
|
rcServer, err := newServer(context.Background(), &opt, http.NewServeMux())
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
original := fs.GetConfig(ctx).UserAgent
|
||||||
|
defer func() { fs.GetConfig(ctx).UserAgent = original }()
|
||||||
|
|
||||||
|
// serveRemote uses s.ctx, marked as an rc request, so global.* must not
|
||||||
|
// change the process-wide config.
|
||||||
|
rcServer.serveRemote(httptest.NewRecorder(), httptest.NewRequest("GET", "/file.txt", nil),
|
||||||
|
"file.txt", ":local,global.user_agent=rcservertest:"+testFs)
|
||||||
|
assert.Equal(t, original, fs.GetConfig(ctx).UserAgent,
|
||||||
|
"the serve path must not let global.* change the process-wide config")
|
||||||
|
}
|
||||||
|
|
||||||
func TestRC(t *testing.T) {
|
func TestRC(t *testing.T) {
|
||||||
tests := []testRun{{
|
tests := []testRun{{
|
||||||
Name: "rc-root",
|
Name: "rc-root",
|
||||||
|
|||||||
Reference in New Issue
Block a user