From 53f972830c1747cb9636a6342c5308e55672c375 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 27 May 2026 10:28:06 +0100 Subject: [PATCH] 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 --- docs/content/rc.md | 11 +++++++++++ fs/config.go | 6 +++++- fs/config_test.go | 18 ++++++++++++++++++ fs/newfs.go | 20 ++++++++++++++++++-- fs/newfs_internal_test.go | 22 ++++++++++++++++++++++ fs/rc/jobs/job.go | 3 +++ fs/rc/jobs/job_test.go | 31 +++++++++++++++++++++++++++++++ fs/rc/rcserver/rcserver.go | 5 +++-- fs/rc/rcserver/rcserver_test.go | 24 ++++++++++++++++++++++++ 9 files changed, 135 insertions(+), 5 deletions(-) diff --git a/docs/content/rc.md b/docs/content/rc.md index 59074c215..6caf64065 100644 --- a/docs/content/rc.md +++ b/docs/content/rc.md @@ -87,6 +87,17 @@ commands or read arbitrary local files. 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 Set this flag to skip reading the modification time (can speed things up). diff --git a/fs/config.go b/fs/config.go index ce0a1df9e..412bec35b 100644 --- a/fs/config.go +++ b/fs/config.go @@ -802,11 +802,15 @@ func GetConfig(ctx context.Context) *ConfigInfo { } // 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 { if srcCtx == nil { return dstCtx } + if IsRCRequest(srcCtx) { + dstCtx = WithRCRequest(dstCtx) + } c := srcCtx.Value(configContextKey) if c == nil { return dstCtx diff --git a/fs/config_test.go b/fs/config_test.go index 141f1185d..4b50b27ff 100644 --- a/fs/config_test.go +++ b/fs/config_test.go @@ -29,3 +29,21 @@ func TestGetConfig(t *testing.T) { config2ctx := GetConfig(ctx2) 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))) +} diff --git a/fs/newfs.go b/fs/newfs.go index 290e4ac40..53a2a0c15 100644 --- a/fs/newfs.go +++ b/fs/newfs.go @@ -25,6 +25,22 @@ var ( 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 // // 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) } Debugf(configName, "Set overridden config %q for backend startup", overrideKeys) - // Set the global context only - if len(globalConfig) != 0 { + // Set the global context unless this Fs is being created for an rc request + if len(globalConfig) != 0 && !IsRCRequest(ctx) { globalCI := GetConfig(context.Background()) err = configstruct.Set(globalConfig, globalCI) if err != nil { diff --git a/fs/newfs_internal_test.go b/fs/newfs_internal_test.go index 6428a5bc8..f064f1286 100644 --- a/fs/newfs_internal_test.go +++ b/fs/newfs_internal_test.go @@ -53,3 +53,25 @@ func TestAddConfigToContext_GlobalOnly(t *testing.T) { ci := GetConfig(newCtx) 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) +} diff --git a/fs/rc/jobs/job.go b/fs/rc/jobs/job.go index 2497764ca..2ba939210 100644 --- a/fs/rc/jobs/job.go +++ b/fs/rc/jobs/job.go @@ -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 ctx = context.WithValue(ctx, jobKey, job) + // Mark the context as created from an rc request + ctx = fs.WithRCRequest(ctx) + if isAsync { go job.run(ctx, fn, in) out = make(rc.Params) diff --git a/fs/rc/jobs/job_test.go b/fs/rc/jobs/job_test.go index e8548fcee..eaf95aead 100644 --- a/fs/rc/jobs/job_test.go +++ b/fs/rc/jobs/job_test.go @@ -300,6 +300,37 @@ func TestExecuteJobWithConfig(t *testing.T) { 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) { ctx := context.Background() called := false diff --git a/fs/rc/rcserver/rcserver.go b/fs/rc/rcserver/rcserver.go index 1c21b98ee..37b3c587f 100644 --- a/fs/rc/rcserver/rcserver.go +++ b/fs/rc/rcserver/rcserver.go @@ -348,7 +348,7 @@ func (s *Server) serveRoot(w http.ResponseWriter, r *http.Request) { // Instantiating a backend from request-supplied configuration can execute // commands during initialisation (e.g. webdav bearer_token_command, sftp ssh), // 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 // 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) 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 { writeError(path, nil, w, fmt.Errorf("failed to make Fs: %w", err), http.StatusInternalServerError) return diff --git a/fs/rc/rcserver/rcserver_test.go b/fs/rc/rcserver/rcserver_test.go index d6ecab461..95f766bfa 100644 --- a/fs/rc/rcserver/rcserver_test.go +++ b/fs/rc/rcserver/rcserver_test.go @@ -480,6 +480,30 @@ func TestServeRemoteWithAuth(t *testing.T) { 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) { tests := []testRun{{ Name: "rc-root",