Skip to content

Commit 410bded

Browse files
committed
fix gosec errors
Signed-off-by: Kim Tsao <[email protected]>
1 parent f0e7f1a commit 410bded

File tree

10 files changed

+91
-26
lines changed

10 files changed

+91
-26
lines changed

index/generator/library/library.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ func CreateIndexFile(index []schema.Schema, indexFilePath string) error {
9191
return fmt.Errorf("failed to marshal %s data: %v", indexFilePath, err)
9292
}
9393

94+
/* #nosec G306 -- index file does not contain any sensitive data*/
9495
err = ioutil.WriteFile(indexFilePath, bytes, 0644)
9596
if err != nil {
9697
return fmt.Errorf("failed to write %s: %v", indexFilePath, err)
@@ -312,6 +313,7 @@ func parseStackDevfile(devfileDirPath string, stackName string, force bool, vers
312313
}
313314
}
314315

316+
/* #nosec G304 -- devfilePath is produced using filepath.Join which cleans the input path */
315317
bytes, err := ioutil.ReadFile(devfilePath)
316318
if err != nil {
317319
return fmt.Errorf("failed to read %s: %v", devfilePath, err)
@@ -403,6 +405,7 @@ func parseStackDevfile(devfileDirPath string, stackName string, force bool, vers
403405
func parseExtraDevfileEntries(registryDirPath string, force bool) ([]schema.Schema, error) {
404406
var index []schema.Schema
405407
extraDevfileEntriesPath := path.Join(registryDirPath, extraDevfileEntries)
408+
/* #nosec G304 -- extraDevfileEntriesPath is produced using path.Join which cleans the input path */
406409
bytes, err := ioutil.ReadFile(extraDevfileEntriesPath)
407410
if err != nil {
408411
return nil, fmt.Errorf("failed to read %s: %v", extraDevfileEntriesPath, err)
@@ -487,6 +490,7 @@ func parseExtraDevfileEntries(registryDirPath string, force bool) ([]schema.Sche
487490
return index, nil
488491
}
489492

493+
/* #nosec G304 -- stackYamlPath is produced from file.Join which cleans the input path */
490494
func parseStackInfo(stackYamlPath string) (schema.Schema, error) {
491495
var index schema.Schema
492496
bytes, err := ioutil.ReadFile(stackYamlPath)

index/generator/library/util.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,25 +123,27 @@ func CloneRemoteStack(git *schema.Git, path string, verbose bool) (err error) {
123123
// returns byte array of zip file and error if occurs otherwise is nil. If git.SubDir is set, then
124124
// zip file will contain contents of the specified subdirectory instead of the whole downloaded git repo.
125125
func DownloadStackFromGit(git *schema.Git, path string, verbose bool) ([]byte, error) {
126-
zipPath := fmt.Sprintf("%s.zip", path)
126+
cleanPath := filepath.Clean(path)
127+
zipPath := fmt.Sprintf("%s.zip", cleanPath)
127128

128129
// Download from given git url. Downloaded result contains subDir
129130
// when specified, if error return empty bytes.
130-
if err := CloneRemoteStack(git, path, verbose); err != nil {
131+
if err := CloneRemoteStack(git, cleanPath, verbose); err != nil {
131132
return []byte{}, err
132133
}
133134

134135
// Throw error if path was not created
135-
if _, err := os.Stat(path); os.IsNotExist(err) {
136+
if _, err := os.Stat(cleanPath); os.IsNotExist(err) {
136137
return []byte{}, err
137138
}
138139

139140
// Zip directory containing downloaded git repo
140-
if err := ZipDir(path, zipPath); err != nil {
141+
if err := ZipDir(cleanPath, zipPath); err != nil {
141142
return []byte{}, err
142143
}
143144

144145
// Read bytes from response and return, error will be nil if successful
146+
/* #nosec G304 -- zipPath is constructed from a clean path */
145147
return ioutil.ReadFile(zipPath)
146148
}
147149

@@ -152,7 +154,7 @@ func DownloadStackFromZipUrl(zipUrl string, subDir string, path string) ([]byte,
152154

153155
// downloadStackFromZipUrl downloads the zip file containing the stack at a given url
154156
func downloadStackFromZipUrl(zipUrl string, subDir string, path string, fs filesystem.Filesystem) ([]byte, error) {
155-
zipDst := fmt.Sprintf("%s.zip", path)
157+
zipDst := fmt.Sprintf("%s.zip", filepath.Clean(path))
156158

157159
// Create path if does not exist
158160
if err := fs.MkdirAll(path, os.ModePerm); err != nil {
@@ -192,6 +194,7 @@ func downloadStackFromZipUrl(zipUrl string, subDir string, path string, fs files
192194
}
193195

194196
// Read bytes from response and return, error will be nil if successful
197+
/* #nosec G304 -- zipDest is produced using a cleaned path */
195198
return ioutil.ReadFile(zipDst)
196199
}
197200

index/server/pkg/server/endpoint.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"net/url"
2626
"os"
2727
"path"
28+
"path/filepath"
2829
"regexp"
2930

3031
"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
@@ -275,7 +276,7 @@ func serveDevfileStarterProjectWithVersion(c *gin.Context) {
275276
localLoc = downloadFilePath
276277
}
277278

278-
downloadBytes, err = ioutil.ReadFile(localLoc)
279+
downloadBytes, err = ioutil.ReadFile(filepath.Clean(localLoc))
279280
if err != nil {
280281
log.Print(err.Error())
281282
c.JSON(http.StatusInternalServerError, gin.H{
@@ -556,6 +557,7 @@ func fetchDevfile(c *gin.Context, name string, version string) ([]byte, indexSch
556557
}
557558
if sampleDevfilePath != "" {
558559
if _, err = os.Stat(sampleDevfilePath); err == nil {
560+
/* #nosec G304 -- sampleDevfilePath is constructed from path.Join which cleans the input paths */
559561
bytes, err = ioutil.ReadFile(sampleDevfilePath)
560562
}
561563
if err != nil {

index/server/pkg/server/registry.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ func pushStackToRegistry(versionComponent indexSchema.Version, stackName string)
6969
if _, err := os.Stat(resourcePath); os.IsNotExist(err) {
7070
resourcePath = filepath.Join(stacksPath, stackName, resource)
7171
}
72+
/* #nosec G304 -- resourcePath is constructed from filepath.Join which cleans the input paths */
7273
resourceContent, err := ioutil.ReadFile(resourcePath)
7374
if err != nil {
7475
return err

index/server/pkg/util/util.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ func IsHtmlRequested(acceptHeader []string) bool {
4545
// EncodeIndexIconToBase64 encodes all index icons to base64 format given the index file path
4646
func EncodeIndexIconToBase64(indexPath string, base64IndexPath string) ([]byte, error) {
4747
// load index
48+
/* #nosec G304 -- indexPath is derived from known paths set in the docker image */
4849
bytes, err := ioutil.ReadFile(indexPath)
4950
if err != nil {
5051
return nil, err
@@ -86,6 +87,7 @@ func encodeToBase64(uri string) (string, error) {
8687
// load the content from the given uri
8788
var bytes []byte
8889
if url.Scheme == "http" || url.Scheme == "https" {
90+
/* #nosec G107 -- uri is taken from the index file. Stacks with URLs to a devile icon should be vetted beforehand */
8991
resp, err := http.Get(uri)
9092
if err != nil {
9193
return "", err
@@ -97,6 +99,7 @@ func encodeToBase64(uri string) (string, error) {
9799
return "", err
98100
}
99101
} else {
102+
/* #nosec G304 -- uri is derived from known paths set in the docker image */
100103
bytes, err = ioutil.ReadFile(uri)
101104
if err != nil {
102105
return "", err
@@ -121,6 +124,7 @@ func encodeToBase64(uri string) (string, error) {
121124
// ReadIndexPath reads the index from the path and unmarshalls it into the index
122125
func ReadIndexPath(indexPath string) ([]indexSchema.Schema, error) {
123126
// load index
127+
/* #nosec G304 -- not user input */
124128
bytes, err := ioutil.ReadFile(indexPath)
125129
if err != nil {
126130
return nil, err

registry-library/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ go 1.14
55
require (
66
github.com/containerd/containerd v1.5.9
77
github.com/devfile/registry-support/index/generator v0.0.0-20220624203950-e7282a4695b6
8+
github.com/hashicorp/go-multierror v1.1.1
89
github.com/hashicorp/go-version v1.4.0
910
github.com/mitchellh/go-homedir v1.1.0
1011
github.com/spf13/cobra v1.2.1

registry-library/go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,13 +511,15 @@ github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFb
511511
github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q=
512512
github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
513513
github.com/hashicorp/errwrap v0.0.0-20141028054710-7554cd9344ce/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
514+
github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=
514515
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
515516
github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
516517
github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60=
517518
github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM=
518519
github.com/hashicorp/go-multierror v0.0.0-20161216184304-ed905158d874/go.mod h1:JMRHfdO9jKNzS/+BTlxCjKNQHg/jZAft8U7LloJvN7I=
519520
github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk=
520521
github.com/hashicorp/go-multierror v1.1.0/go.mod h1:spPvp8C1qA32ftKqdAHm4hHTbPw+vmowP0z+KUhOZdA=
522+
github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
521523
github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
522524
github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z+hiSfzSTQa6eBIzfwKfwNnHU=
523525
github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU=

registry-library/library/library.go

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"archive/zip"
2020
"encoding/json"
2121
"fmt"
22+
"github.com/hashicorp/go-multierror"
2223
"io"
2324
"io/ioutil"
2425
"net/http"
@@ -314,11 +315,12 @@ func DownloadStarterProjectAsDir(path string, registryURL string, stack string,
314315
defer archive.Close()
315316

316317
// Extract files from starter project archive to specified directory path
318+
cleanPath := filepath.Clean(path)
317319
for _, file := range archive.File {
318-
filePath := filepath.Join(path, file.Name)
320+
filePath := filepath.Join(cleanPath, filepath.Clean(file.Name))
319321

320322
// validate extracted filepath
321-
if filePath != file.Name && !strings.HasPrefix(filePath, filepath.Clean(path)+string(os.PathSeparator)) {
323+
if filePath != file.Name && !strings.HasPrefix(filePath, cleanPath+string(os.PathSeparator)) {
322324
return fmt.Errorf("invalid file path %s", filePath)
323325
}
324326

@@ -334,6 +336,7 @@ func DownloadStarterProjectAsDir(path string, registryURL string, stack string,
334336
}
335337

336338
// open destination file
339+
/* #nosec G304 -- filePath is produced using path.Join which cleans the dir path */
337340
dstFile, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, file.Mode())
338341
if err != nil {
339342
return fmt.Errorf("error opening destination file at %s: %v", filePath, err)
@@ -346,12 +349,19 @@ func DownloadStarterProjectAsDir(path string, registryURL string, stack string,
346349
}
347350

348351
// extract source file to destination file
352+
/* #nosec G110 -- starter projects are vetted before they are added to a registry. Their contents can be seen before they are downloaded */
349353
if _, err = io.Copy(dstFile, srcFile); err != nil {
350354
return fmt.Errorf("error extracting file %s from archive %s to destination at %s: %v", file.Name, archivePath, filePath, err)
351355
}
352356

353-
dstFile.Close()
354-
srcFile.Close()
357+
err = dstFile.Close()
358+
if err != nil {
359+
return err
360+
}
361+
err = srcFile.Close()
362+
if err != nil {
363+
return err
364+
}
355365
}
356366

357367
return nil
@@ -360,37 +370,45 @@ func DownloadStarterProjectAsDir(path string, registryURL string, stack string,
360370
// DownloadStarterProject downloads a specified starter project archive to a given path
361371
func DownloadStarterProject(path string, registryURL string, stack string, starterProject string, options RegistryOptions) error {
362372
var fileStream *os.File
373+
var returnedErr error
363374

375+
cleanPath := filepath.Clean(path)
364376
// Download Starter Project archive bytes
365377
bytes, err := DownloadStarterProjectAsBytes(registryURL, stack, starterProject, options)
366378
if err != nil {
367379
return err
368380
}
369381

370382
// Error if parent directory does not exist
371-
if _, err = os.Stat(filepath.Dir(path)); os.IsNotExist(err) {
383+
if _, err = os.Stat(filepath.Dir(cleanPath)); os.IsNotExist(err) {
372384
return fmt.Errorf("parent directory '%s' does not exist: %v", filepath.Dir(path), err)
373385
}
374386

375387
// If file does not exist, create a new one
376388
// Else open existing for overwriting
377389
if _, err = os.Stat(path); os.IsNotExist(err) {
378-
fileStream, err = os.Create(path)
390+
fileStream, err = os.Create(cleanPath)
379391
if err != nil {
380392
return fmt.Errorf("failed to create file '%s': %v", path, err)
381393
}
382394
} else {
383-
fileStream, err = os.OpenFile(path, os.O_WRONLY|os.O_TRUNC, os.ModePerm)
395+
fileStream, err = os.OpenFile(cleanPath, os.O_WRONLY|os.O_TRUNC, os.ModePerm)
384396
if err != nil {
385397
return fmt.Errorf("failed to open file '%s': %v", path, err)
386398
}
387399
}
388-
defer fileStream.Close()
400+
401+
defer func() {
402+
if err = fileStream.Close(); err != nil {
403+
returnedErr = multierror.Append(returnedErr, err)
404+
}
405+
}()
389406

390407
// Write downloaded bytes to file
391408
_, err = fileStream.Write(bytes)
392409
if err != nil {
393-
return fmt.Errorf("failed writing to '%s': %v", path, err)
410+
returnedErr = multierror.Append(returnedErr, fmt.Errorf("failed writing to '%s': %v", path, err))
411+
return returnedErr
394412
}
395413

396414
return nil

registry-library/library/util.go

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"compress/gzip"
2121
"crypto/tls"
2222
"fmt"
23+
"github.com/hashicorp/go-multierror"
2324
"io"
2425
"log"
2526
"net/http"
@@ -73,47 +74,70 @@ func ValidateStackVersionTag(stackWithVersion string) (bool, error) {
7374

7475
// decompress extracts the archive file
7576
func decompress(targetDir string, tarFile string, excludeFiles []string) error {
76-
reader, err := os.Open(tarFile)
77+
var returnedErr error
78+
79+
reader, err := os.Open(filepath.Clean(tarFile))
7780
if err != nil {
7881
return err
7982
}
80-
defer reader.Close()
83+
84+
defer func() {
85+
if err = reader.Close(); err != nil {
86+
returnedErr = multierror.Append(returnedErr, err)
87+
}
88+
}()
8189

8290
gzReader, err := gzip.NewReader(reader)
8391
if err != nil {
84-
return err
92+
returnedErr = multierror.Append(returnedErr, err)
93+
return returnedErr
8594
}
86-
defer gzReader.Close()
95+
96+
defer func() {
97+
if err = gzReader.Close(); err != nil {
98+
returnedErr = multierror.Append(returnedErr, err)
99+
}
100+
}()
87101

88102
tarReader := tar.NewReader(gzReader)
89103
for {
90104
header, err := tarReader.Next()
91105
if err == io.EOF {
92106
break
93107
} else if err != nil {
94-
return err
108+
returnedErr = multierror.Append(returnedErr, err)
109+
return returnedErr
95110
}
96111
if isExcluded(header.Name, excludeFiles) {
97112
continue
98113
}
99114

100-
target := path.Join(targetDir, header.Name)
115+
target := path.Join(targetDir, filepath.Clean(header.Name))
101116
switch header.Typeflag {
102117
case tar.TypeDir:
103118
err = os.MkdirAll(target, os.FileMode(header.Mode))
104119
if err != nil {
105-
return err
120+
returnedErr = multierror.Append(returnedErr, err)
121+
return returnedErr
106122
}
107123
case tar.TypeReg:
124+
/* #nosec G304 -- target is produced using path.Join which cleans the dir path */
108125
w, err := os.OpenFile(target, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode))
109126
if err != nil {
110-
return err
127+
returnedErr = multierror.Append(returnedErr, err)
128+
return returnedErr
111129
}
130+
/* #nosec G110 -- starter projects are vetted before they are added to a registry. Their contents can be seen before they are downloaded */
112131
_, err = io.Copy(w, tarReader)
113132
if err != nil {
114-
return err
133+
returnedErr = multierror.Append(returnedErr, err)
134+
return returnedErr
135+
}
136+
err = w.Close()
137+
if err != nil {
138+
returnedErr = multierror.Append(returnedErr, err)
139+
return returnedErr
115140
}
116-
w.Close()
117141
default:
118142
log.Printf("Unsupported type: %v", header.Typeflag)
119143
}
@@ -161,7 +185,8 @@ func getHTTPClient(options RegistryOptions) *http.Client {
161185
Transport: &http.Transport{
162186
Proxy: http.ProxyFromEnvironment,
163187
ResponseHeaderTimeout: overriddenTimeout,
164-
TLSClientConfig: &tls.Config{InsecureSkipVerify: options.SkipTLSVerify},
188+
/*#nosec G402 -- documented user option for dev/test, not for prod use */
189+
TLSClientConfig: &tls.Config{InsecureSkipVerify: options.SkipTLSVerify},
165190
},
166191
Timeout: overriddenTimeout,
167192
}

registry-library/vendor/modules.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,11 @@ github.com/google/gofuzz
9898
github.com/google/gofuzz/bytesource
9999
# github.com/gorilla/mux v1.8.0
100100
github.com/gorilla/mux
101+
# github.com/hashicorp/errwrap v1.0.0
102+
github.com/hashicorp/errwrap
103+
# github.com/hashicorp/go-multierror v1.1.1
104+
## explicit
105+
github.com/hashicorp/go-multierror
101106
# github.com/hashicorp/go-version v1.4.0
102107
## explicit
103108
github.com/hashicorp/go-version

0 commit comments

Comments
 (0)