Skip to content

Use SafeJoin in V2 Tools Install endpoint #840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 2, 2023
+103 −79
Merged

Use SafeJoin in V2 Tools Install endpoint #840

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6ff8768
add testcase and un-export vars
umbynos Sep 29, 2023
8433390
removing test: already tested (better) in `test_main.go`
umbynos Sep 29, 2023
da8e8b4
move test to a different function and add testcase for install endpoint
umbynos Sep 29, 2023
cdac540
use SafeJoin on tools Install handler
umbynos Sep 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions main_test.go
View file
Original file line number Diff line number Diff line change
@@ -98,26 +98,36 @@ func TestInstallToolV2(t *testing.T) {
responseBody string
}

BossacURL := "http://downloads.arduino.cc/tools/bossac-1.7.0-arduino3-linux64.tar.gz"
BossacChecksum := "SHA-256:1ae54999c1f97234a5c603eb99ad39313b11746a4ca517269a9285afa05f9100"
BossacSignature := "382898a97b5a86edd74208f10107d2fecbf7059ffe9cc856e045266fb4db4e98802728a0859cfdcda1c0b9075ec01e42dbea1f430b813530d5a6ae1766dfbba64c3e689b59758062dc2ab2e32b2a3491dc2b9a80b9cda4ae514fbe0ec5af210111b6896976053ab76bac55bcecfcececa68adfa3299e3cde6b7f117b3552a7d80ca419374bb497e3c3f12b640cf5b20875416b45e662fc6150b99b178f8e41d6982b4c0a255925ea39773683f9aa9201dc5768b6fc857c87ff602b6a93452a541b8ec10ca07f166e61a9e9d91f0a6090bd2038ed4427af6251039fb9fe8eb62ec30d7b0f3df38bc9de7204dec478fb86f8eb3f71543710790ee169dce039d3e0"
bossacURL := "http://downloads.arduino.cc/tools/bossac-1.7.0-arduino3-linux64.tar.gz"
bossacChecksum := "SHA-256:1ae54999c1f97234a5c603eb99ad39313b11746a4ca517269a9285afa05f9100"
bossacSignature := "382898a97b5a86edd74208f10107d2fecbf7059ffe9cc856e045266fb4db4e98802728a0859cfdcda1c0b9075ec01e42dbea1f430b813530d5a6ae1766dfbba64c3e689b59758062dc2ab2e32b2a3491dc2b9a80b9cda4ae514fbe0ec5af210111b6896976053ab76bac55bcecfcececa68adfa3299e3cde6b7f117b3552a7d80ca419374bb497e3c3f12b640cf5b20875416b45e662fc6150b99b178f8e41d6982b4c0a255925ea39773683f9aa9201dc5768b6fc857c87ff602b6a93452a541b8ec10ca07f166e61a9e9d91f0a6090bd2038ed4427af6251039fb9fe8eb62ec30d7b0f3df38bc9de7204dec478fb86f8eb3f71543710790ee169dce039d3e0"
bossacInstallURLOK := tools.ToolPayload{
Name: "bossac",
Version: "1.7.0-arduino3",
Packager: "arduino",
URL: &BossacURL,
Checksum: &BossacChecksum,
Signature: &BossacSignature,
URL: &bossacURL,
Checksum: &bossacChecksum,
Signature: &bossacSignature,
}

WrongSignature := "wr0ngs1gn4tur3"
wrongSignature := "wr0ngs1gn4tur3"
bossacInstallWrongSig := tools.ToolPayload{
Name: "bossac",
Version: "1.7.0-arduino3",
Packager: "arduino",
URL: &BossacURL,
Checksum: &BossacChecksum,
Signature: &WrongSignature,
URL: &bossacURL,
Checksum: &bossacChecksum,
Signature: &wrongSignature,
}

wrongChecksum := "wr0ngch3cksum"
bossacInstallWrongCheck := tools.ToolPayload{
Name: "bossac",
Version: "1.7.0-arduino3",
Packager: "arduino",
URL: &bossacURL,
Checksum: &wrongChecksum,
Signature: &bossacSignature,
}

bossacInstallNoURL := tools.ToolPayload{
@@ -129,6 +139,7 @@ func TestInstallToolV2(t *testing.T) {
tests := []test{
{bossacInstallURLOK, http.StatusOK, "ok"},
{bossacInstallWrongSig, http.StatusInternalServerError, "verification error"},
{bossacInstallWrongCheck, http.StatusInternalServerError, "checksum doesn't match"},
{bossacInstallNoURL, http.StatusBadRequest, "tool not found"}, //because the index is not added
}

27 changes: 20 additions & 7 deletions v2/pkgs/tools.go
View file
Original file line number Diff line number Diff line change
@@ -191,23 +191,28 @@ func (c *Tools) install(ctx context.Context, path, url, checksum string) (*tools
var buffer bytes.Buffer
reader := io.TeeReader(res.Body, &buffer)

safePath, err := utilities.SafeJoin(c.Folder, path)
if err != nil {
return nil, err
}

// Cleanup
err = os.RemoveAll(filepath.Join(c.Folder, path))
err = os.RemoveAll(safePath)
if err != nil {
return nil, err
}

err = extract.Archive(ctx, reader, c.Folder, rename(path))
if err != nil {
os.RemoveAll(path)
os.RemoveAll(safePath)
return nil, err
}

sum := sha256.Sum256(buffer.Bytes())
sumString := "SHA-256:" + hex.EncodeToString(sum[:sha256.Size])

if sumString != checksum {
os.RemoveAll(path)
os.RemoveAll(safePath)
return nil, errors.New("checksum doesn't match")
}

@@ -249,7 +254,11 @@ func writeInstalled(folder, path string) error {
// read installed.json
installed := map[string]string{}

data, err := os.ReadFile(filepath.Join(folder, "installed.json"))
installedFile, err := utilities.SafeJoin(folder, "installed.json")
if err != nil {
return err
}
data, err := os.ReadFile(installedFile)
if err == nil {
err = json.Unmarshal(data, &installed)
if err != nil {
@@ -260,13 +269,17 @@ func writeInstalled(folder, path string) error {
parts := strings.Split(path, string(filepath.Separator))
tool := parts[len(parts)-2]
toolWithVersion := fmt.Sprint(tool, "-", parts[len(parts)-1])
installed[tool] = filepath.Join(folder, path)
installed[toolWithVersion] = filepath.Join(folder, path)
toolFile, err := utilities.SafeJoin(folder, path)
if err != nil {
return err
}
installed[tool] = toolFile
installed[toolWithVersion] = toolFile

data, err = json.Marshal(installed)
if err != nil {
return err
}

return os.WriteFile(filepath.Join(folder, "installed.json"), data, 0644)
return os.WriteFile(installedFile, data, 0644)
}
124 changes: 62 additions & 62 deletions v2/pkgs/tools_test.go
View file
Original file line number Diff line number Diff line change
@@ -131,79 +131,79 @@ func TestTools(t *testing.T) {
if len(installed) != 0 {
t.Fatalf("expected %d == %d (%s)", len(installed), 0, "len(installed)")
}
}

// Install a tool by specifying url and checksum
_, err = service.Install(ctx, &tools.ToolPayload{
Packager: "arduino",
Name: "avrdude",
Version: "6.0.1-arduino2",
URL: strpoint(url()),
Checksum: strpoint(checksum()),
})
if err != nil {
t.Fatal(err)
func TestEvilFilename(t *testing.T) {

// Initialize indexes with a temp folder
tmp := t.TempDir()

service := pkgs.Tools{
Folder: tmp,
Indexes: &pkgs.Indexes{
Folder: tmp,
},
}

installed, err = service.Installed(ctx)
if err != nil {
t.Fatal(err)
ctx := context.Background()

type test struct {
fileName string
errBody string
}

evilFileNames := []string{
"/",
"..",
"../",
"../evil.txt",
"../../../../../../../../../../../../../../../../../../../../tmp/evil.txt",
"some/path/../../../../../../../../../../../../../../../../../../../../tmp/evil.txt",
}
if runtime.GOOS == "windows" {
evilFileNames = []string{
"..\\",
"..\\evil.txt",
"..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\tmp\\evil.txt",
"some\\path\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\tmp\\evil.txt",
}
}
if len(installed) != 1 {
t.Fatalf("expected %d == %d (%s)", len(installed), 1, "len(installed)")
tests := []test{}
for _, evilFileName := range evilFileNames {
tests = append(tests, test{fileName: evilFileName,
errBody: "unsafe path join"})
}

t.Run("payload containing evil names", func(t *testing.T) {
evilFileNames := []string{
"/",
"..",
"../",
"../evil.txt",
"../../../../../../../../../../../../../../../../../../../../tmp/evil.txt",
"some/path/../../../../../../../../../../../../../../../../../../../../tmp/evil.txt",
}
if runtime.GOOS == "windows" {
evilFileNames = []string{
"..\\",
"..\\evil.txt",
"..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\tmp\\evil.txt",
"some\\path\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\tmp\\evil.txt",
}
}
for _, evilFileName := range evilFileNames {
toolsTemplate := tools.ToolPayload{
// We'll replace the name directly in the test
Checksum: strpoint("SHA-256:1ae54999c1f97234a5c603eb99ad39313b11746a4ca517269a9285afa05f9100"),
Signature: strpoint("382898a97b5a86edd74208f10107d2fecbf7059ffe9cc856e045266fb4db4e98802728a0859cfdcda1c0b9075ec01e42dbea1f430b813530d5a6ae1766dfbba64c3e689b59758062dc2ab2e32b2a3491dc2b9a80b9cda4ae514fbe0ec5af210111b6896976053ab76bac55bcecfcececa68adfa3299e3cde6b7f117b3552a7d80ca419374bb497e3c3f12b640cf5b20875416b45e662fc6150b99b178f8e41d6982b4c0a255925ea39773683f9aa9201dc5768b6fc857c87ff602b6a93452a541b8ec10ca07f166e61a9e9d91f0a6090bd2038ed4427af6251039fb9fe8eb62ec30d7b0f3df38bc9de7204dec478fb86f8eb3f71543710790ee169dce039d3e0"),
URL: strpoint("http://downloads.arduino.cc/tools/bossac-1.7.0-arduino3-linux64.tar.gz"),
}

for _, test := range tests {
t.Run("REMOVE payload containing evil names: "+test.fileName, func(t *testing.T) {
// Here we could inject malicious name also in the Packager and Version field.
// Since the path is made by joining all of these 3 fields, we're using only the Name,
// as it won't change the result and let us keep the test small and readable.
_, err := service.Remove(ctx, &tools.ToolPayload{Name: evilFileName})
require.Error(t, err, evilFileName)
require.ErrorContains(t, err, "unsafe path join")
}
})
_, err := service.Remove(ctx, &tools.ToolPayload{Name: test.fileName})
require.Error(t, err, test)
require.ErrorContains(t, err, test.errBody)
})
}
for _, test := range tests {
toolsTemplate.Name = test.fileName
t.Run("INSTALL payload containing evil names: "+toolsTemplate.Name, func(t *testing.T) {
// Here we could inject malicious name also in the Packager and Version field.
// Since the path is made by joining all of these 3 fields, we're using only the Name,
// as it won't change the result and let us keep the test small and readable.
_, err := service.Install(ctx, &toolsTemplate)
require.Error(t, err, test)
require.ErrorContains(t, err, test.errBody)
})
}
}

func strpoint(s string) *string {
return &s
}

func url() string {
urls := map[string]string{
"linuxamd64": "https://downloads.arduino.cc/tools/avrdude-6.0.1-arduino2-x86_64-pc-linux-gnu.tar.bz2",
"linux386": "https://downloads.arduino.cc/tools/avrdude-6.0.1-arduino2-i686-pc-linux-gnu.tar.bz2",
"darwinamd64": "https://downloads.arduino.cc/tools/avrdude-6.0.1-arduino2-i386-apple-darwin11.tar.bz2",
"windows386": "https://downloads.arduino.cc/tools/avrdude-6.0.1-arduino2-i686-mingw32.zip",
"windowsamd64": "https://downloads.arduino.cc/tools/avrdude-6.0.1-arduino2-i686-mingw32.zip",
}

return urls[runtime.GOOS+runtime.GOARCH]
}

func checksum() string {
checksums := map[string]string{
"linuxamd64": "SHA-256:2489004d1d98177eaf69796760451f89224007c98b39ebb5577a9a34f51425f1",
"linux386": "SHA-256:6f633dd6270ad0d9ef19507bcbf8697b414a15208e4c0f71deec25ef89cdef3f",
"darwinamd64": "SHA-256:71117cce0096dad6c091e2c34eb0b9a3386d3aec7d863d2da733d9e5eac3a6b1",
"windows386": "SHA-256:6c5483800ba753c80893607e30cade8ab77b182808fcc5ea15fa3019c63d76ae",
"windowsamd64": "SHA-256:6c5483800ba753c80893607e30cade8ab77b182808fcc5ea15fa3019c63d76ae",
}
return checksums[runtime.GOOS+runtime.GOARCH]

}