diff --git a/arduino/resources/helpers.go b/arduino/resources/helpers.go index c735ff36559..e6251bd0192 100644 --- a/arduino/resources/helpers.go +++ b/arduino/resources/helpers.go @@ -20,6 +20,7 @@ import ( "os" "github.com/arduino/go-paths-helper" + "github.com/pkg/errors" "go.bug.st/downloader/v2" ) @@ -30,7 +31,18 @@ func (r *DownloadResource) ArchivePath(downloadDir *paths.Path) (*paths.Path, er if err := staging.MkdirAll(); err != nil { return nil, err } - return staging.Join(r.ArchiveFileName), nil + + // Filter out paths from file name + archiveFile, err := paths.SafeNew(r.ArchiveFileName) + if err != nil { + return nil, errors.Errorf("invalid filename: %s", r.ArchiveFileName) + } + archiveFileName := archiveFile.Base() + archivePath := staging.Join(archiveFileName).Clean() + if archivePath.IsDir() { + return nil, errors.Errorf("archive filename points to an existing directory: %s", archivePath) + } + return archivePath, nil } // IsCached returns true if the specified DownloadResource has already been downloaded @@ -44,6 +56,11 @@ func (r *DownloadResource) IsCached(downloadDir *paths.Path) (bool, error) { // Download a DownloadResource. func (r *DownloadResource) Download(downloadDir *paths.Path, config *downloader.Config) (*downloader.Downloader, error) { + path, err := r.ArchivePath(downloadDir) + if err != nil { + return nil, fmt.Errorf("getting archive path: %s", err) + } + cached, err := r.TestLocalArchiveIntegrity(downloadDir) if err != nil { return nil, fmt.Errorf("testing local archive integrity: %s", err) @@ -53,11 +70,6 @@ func (r *DownloadResource) Download(downloadDir *paths.Path, config *downloader. return nil, nil } - path, err := r.ArchivePath(downloadDir) - if err != nil { - return nil, fmt.Errorf("getting archive path: %s", err) - } - if stats, err := path.Stat(); os.IsNotExist(err) { // normal download } else if err == nil && stats.Size() > r.Size { diff --git a/arduino/resources/helpers_test.go b/arduino/resources/helpers_test.go index 21445d25aab..5d6c3987439 100644 --- a/arduino/resources/helpers_test.go +++ b/arduino/resources/helpers_test.go @@ -36,6 +36,94 @@ func (h *EchoHandler) ServeHTTP(writer http.ResponseWriter, request *http.Reques request.Write(writer) } +func TestResourcesSanityChecks(t *testing.T) { + tmp, err := paths.MkTempDir("", "") + require.NoError(t, err) + defer tmp.RemoveAll() + + { + testArchiveFileNames := []string{ + "test.txt", + "/test.txt", + "somepath/to/test.txt", + "/somepath/to/test.txt", + "path/to/../test.txt", + "/path/to/../test.txt", + "../test.txt", + "/../test.txt", + } + for _, testArchiveFileName := range testArchiveFileNames { + r := &DownloadResource{ + ArchiveFileName: testArchiveFileName, + CachePath: "cache", + } + archivePath, err := r.ArchivePath(tmp) + require.NoError(t, err) + expectedArchivePath := tmp.Join("cache", "test.txt") + require.Equal(t, expectedArchivePath.String(), archivePath.String()) + } + } + + { + r := &DownloadResource{ + ArchiveFileName: "/test.txt", + CachePath: "cache", + } + archivePath, err := r.ArchivePath(tmp) + require.NoError(t, err) + expectedArchivePath := tmp.Join("cache", "test.txt") + require.Equal(t, expectedArchivePath.String(), archivePath.String()) + } + + { + testArchiveFileNames := []string{ + "/", + ".", + "/.", + "..", + "/..", + "path/..", + "/path/..", + "path/path/..", + "/path/path/..", + ".." + string([]byte{0xC0, 0xAF}) + "test.txt", + "/.." + string([]byte{0xC0, 0xAF}) + "test.txt", + } + for _, testArchiveFileName := range testArchiveFileNames { + r := &DownloadResource{ + ArchiveFileName: testArchiveFileName, + CachePath: "cache", + } + archivePath, err := r.ArchivePath(tmp) + require.Nil(t, archivePath) + require.Error(t, err) + } + } +} + +func TestResourceErrorHandling(t *testing.T) { + tmp, err := paths.MkTempDir("", "") + require.NoError(t, err) + defer tmp.RemoveAll() + + r := &DownloadResource{ + ArchiveFileName: "..", + CachePath: "cache", + } + + c, err := r.IsCached(tmp) + require.Error(t, err) + require.False(t, c) + + d, err := r.Download(tmp, nil) + require.Error(t, err) + require.Nil(t, d) + + e, err := r.TestLocalArchiveIntegrity(tmp) + require.Error(t, err) + require.False(t, e) +} + func TestDownloadApplyUserAgentHeaderUsingConfig(t *testing.T) { goldUserAgentValue := fmt.Sprintf("arduino-cli/0.0.0-test.preview (amd64; linux; go1.12.4) Commit:deadbeef/Build:2019-06-12 11:11:11.111") goldUserAgentString := "User-Agent: " + goldUserAgentValue diff --git a/arduino/resources/resources_test.go b/arduino/resources/resources_test.go index 326f34c11f3..f458171fe8b 100644 --- a/arduino/resources/resources_test.go +++ b/arduino/resources/resources_test.go @@ -80,14 +80,26 @@ func TestDownloadAndChecksums(t *testing.T) { r.Checksum = "BOH:12312312312313123123123123123123" _, err = r.TestLocalArchiveChecksum(tmp) require.Error(t, err) + _, err = r.TestLocalArchiveIntegrity(tmp) + require.Error(t, err) + _, err = r.Download(tmp, nil) + require.Error(t, err) r.Checksum = "MD5 667cf48afcc12c38c8c1637947a04224" _, err = r.TestLocalArchiveChecksum(tmp) require.Error(t, err) + _, err = r.TestLocalArchiveIntegrity(tmp) + require.Error(t, err) + _, err = r.Download(tmp, nil) + require.Error(t, err) r.Checksum = "MD5:zmxcbzxmncbzxmnbczxmnbczxmnbcnnb" _, err = r.TestLocalArchiveChecksum(tmp) require.Error(t, err) + _, err = r.TestLocalArchiveIntegrity(tmp) + require.Error(t, err) + _, err = r.Download(tmp, nil) + require.Error(t, err) r.Checksum = "SHA-1:c007e47637cc6ad6176e7d94aeffc232ee34c1c1" res, err := r.TestLocalArchiveChecksum(tmp) diff --git a/go.mod b/go.mod index a07ab1e2cfd..7cd566949cf 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( bou.ke/monkey v1.0.1 github.com/GeertJohan/go.rice v1.0.0 github.com/arduino/board-discovery v0.0.0-20180823133458-1ba29327fb0c - github.com/arduino/go-paths-helper v1.2.0 + github.com/arduino/go-paths-helper v1.2.1-0.20200802112116-33dcc69b14ba github.com/arduino/go-properties-orderedmap v1.3.0 github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b github.com/arduino/go-win32-utils v0.0.0-20180330194947-ed041402e83b diff --git a/go.sum b/go.sum index 30933947a33..87f959e7399 100644 --- a/go.sum +++ b/go.sum @@ -18,6 +18,8 @@ github.com/arduino/go-paths-helper v1.0.1 h1:utYXLM2RfFlc9qp/MJTIYp3t6ux/xM6mWje github.com/arduino/go-paths-helper v1.0.1/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck= github.com/arduino/go-paths-helper v1.2.0 h1:qDW93PR5IZUN/jzO4rCtexiwF8P4OIcOmcSgAYLZfY4= github.com/arduino/go-paths-helper v1.2.0/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck= +github.com/arduino/go-paths-helper v1.2.1-0.20200802112116-33dcc69b14ba h1:rQtLTpIICgc8ad2UG/A7X1F4TpKGoazBxhKR+crsf4k= +github.com/arduino/go-paths-helper v1.2.1-0.20200802112116-33dcc69b14ba/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck= github.com/arduino/go-properties-orderedmap v1.3.0 h1:4No/vQopB36e7WUIk6H6TxiSEJPiMrVOCZylYmua39o= github.com/arduino/go-properties-orderedmap v1.3.0/go.mod h1:DKjD2VXY/NZmlingh4lSFMEYCVubfeArCsGPGDwb2yk= github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b h1:9hDi4F2st6dbLC3y4i02zFT5quS4X6iioWifGlVwfy4=