diff --git a/main_test.go b/main_test.go index 591cba1d7..c2fb2489e 100644 --- a/main_test.go +++ b/main_test.go @@ -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 } diff --git a/v2/pkgs/tools.go b/v2/pkgs/tools.go index c153884cb..2cb92553c 100644 --- a/v2/pkgs/tools.go +++ b/v2/pkgs/tools.go @@ -191,15 +191,20 @@ 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 } @@ -207,7 +212,7 @@ func (c *Tools) install(ctx context.Context, path, url, checksum string) (*tools 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) } diff --git a/v2/pkgs/tools_test.go b/v2/pkgs/tools_test.go index 70236cff3..04198cb2f 100644 --- a/v2/pkgs/tools_test.go +++ b/v2/pkgs/tools_test.go @@ -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] - -}