Files
syncthing/lib/versioner/external_test.go
Jakob Borg 97cb72a608 chore(versioner): attempt to prevent blatantly unsafe external versioner commands (fixes #10721) (#10722)
While preparing the command, attempt to verify that the template
expansion happens in a way that will result in a non-shell-injection
command. I don't presume to say that this is a 100% prevention, and the
script itself can always do dumb shit with the file path later.
Nonetheless, we should make a best-effort attempt.

Equally, this could generate false positives for commands that are
strangely written but in fact safe. I think this is acceptable; external
versioning is currently used by approximately 0.02% of users, and
presumably most of them have a setup that is sane.

---------

Signed-off-by: Jakob Borg <jakob@kastelo.net>
2026-06-23 07:48:53 +02:00

124 lines
3.0 KiB
Go

// Copyright (C) 2016 The Syncthing Authors.
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.
package versioner
import (
"os"
"path/filepath"
"testing"
"github.com/syncthing/syncthing/lib/build"
"github.com/syncthing/syncthing/lib/fs"
)
func TestExternalNoCommand(t *testing.T) {
file := "testdata/folder path/long filename.txt"
prepForRemoval(t, file)
defer os.RemoveAll("testdata")
// The file should exist before the versioner run.
if _, err := os.Lstat(file); err != nil {
t.Fatal("File should exist")
}
// The versioner should fail due to missing command.
e := external{
filesystem: fs.NewFilesystem(fs.FilesystemTypeBasic, "."),
command: "nonexistent command",
}
if err := e.Archive(file); err == nil {
t.Error("Command should have failed")
}
// The file should not have been removed.
if _, err := os.Lstat(file); err != nil {
t.Fatal("File should still exist")
}
}
func TestExternal(t *testing.T) {
cmd := "./_external_test/external.sh %FOLDER_PATH% %FILE_PATH%"
if build.IsWindows {
cmd = `.\\_external_test\\external.bat %FOLDER_PATH% %FILE_PATH%`
}
file := filepath.Join("testdata", "folder path", "dir (parens)", "/long filename (parens).txt")
prepForRemoval(t, file)
defer os.RemoveAll("testdata")
// The file should exist before the versioner run.
if _, err := os.Lstat(file); err != nil {
t.Fatal("File should exist")
}
// The versioner should run successfully.
e := external{
filesystem: fs.NewFilesystem(fs.FilesystemTypeBasic, "."),
command: cmd,
}
if err := e.Archive(file); err != nil {
t.Fatal(err)
}
// The file should no longer exist.
if _, err := os.Lstat(file); !os.IsNotExist(err) {
t.Error("File should no longer exist")
}
}
func TestExternalCommandSplit(t *testing.T) {
e := external{
filesystem: fs.NewFilesystem(fs.FilesystemTypeFake, "TestExternalCommandSplit"),
}
cases := []struct {
cmd string
safe bool
}{
{`echo %FOLDER_PATH% %FILE_PATH%`, true},
{`echo "%FOLDER_PATH% %FILE_PATH%"`, false},
{`echo %FOLDER_PATH%/%FILE_PATH%`, true},
{`echo "%FOLDER_PATH%/%FILE_PATH%"`, true},
{`echo '%FOLDER_PATH%/%FILE_PATH%'`, true},
{`echo "'%FOLDER_PATH%/%FILE_PATH%'"`, false},
{`sh -c "echo '%FOLDER_PATH%/%FILE_PATH%'"`, false},
{`sh -c "echo %FOLDER_PATH%/%FILE_PATH%"`, false},
}
for _, tc := range cases {
e.command = tc.cmd
res, err := e.prepareCommand("evil file name")
if tc.safe && err != nil {
t.Fatal(err)
}
if !tc.safe && err == nil {
t.Logf("%q", res.Path)
t.Logf("%q", res.Args)
t.Errorf("should be unsafe: %q", tc.cmd)
}
}
}
func prepForRemoval(t *testing.T, file string) {
if err := os.RemoveAll("testdata"); err != nil {
t.Fatal(err)
}
if err := os.MkdirAll(filepath.Dir(file), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(file, []byte("hello\n"), 0o644); err != nil {
t.Fatal(err)
}
}