Merge pull request #48 from keep-network/add-gosec

Add GoSec workflow

Here we introduce the GoSec scan workflow to the Github Actions
pipeline and deal with all discovered problems.

.github/workflows/gosec.yml

@@ -0,0 +1,20 @@
+name: Gosec
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  scan:
+    runs-on: ubuntu-latest
+    env:
+      GO111MODULE: on
+    steps:
+      - uses: actions/checkout@v2
+      - uses: securego/gosec@master
+        with:
+          args: ./...

pkg/chain/ethereum/ethutil/ethutil.go

@@ -35,6 +35,8 @@ func AddressFromHex(hex string) (common.Address, error) {
 
 // DecryptKeyFile reads in a key file and uses the password to decrypt it.
 func DecryptKeyFile(keyFile, password string) (*keystore.Key, error) {
+	// #nosec G304 (file path provided as taint input)
+	// This line is used to read a local key file. There is no user input.
 	data, err := ioutil.ReadFile(keyFile)
 	if err != nil {
 		return nil, fmt.Errorf("unable to read KeyFile %s [%v]", keyFile, err)
@@ -141,7 +143,7 @@ func CallAtBlock(
 // the true gas limit requirement as other transactions may be added or removed by miners,
 // but it should provide a basis for setting a reasonable default.
 func EstimateGas(
-	from common.Address, 
+	from common.Address,
 	to common.Address,
 	method string,
 	contractABI *abi.ABI,

pkg/generate/generate.go

@@ -38,6 +38,12 @@ func OrganizeImports(codeBuffer *bytes.Buffer, filePath string) error {
 // error writing the file.
 func SaveBufferToFile(buffer *bytes.Buffer, filePath string) error {
 	file, err := os.Create(filePath)
+
+	// #nosec G104 G307 (audit errors not checked & deferring unsafe method)
+	// This line is placed in the auxiliary generator code,
+	// not in the core application. Also, the Close function returns only
+	// the error. It doesn't return any other values which can be a security
+	// threat when used without checking the error.
 	defer file.Close()
 	if err != nil {
 		return fmt.Errorf("output file %s creation failed [%v]", filePath, err)

pkg/persistence/disk_persistence.go

@@ -123,26 +123,32 @@ func write(filePath string, data []byte) error {
 		return err
 	}
 
-	defer writeFile.Close()
+	defer closeFile(writeFile)
 
 	_, err = writeFile.Write(data)
 	if err != nil {
 		return err
 	}
 
-	writeFile.Sync()
+	err = writeFile.Sync()
+	if err != nil {
+		return err
+	}
 
 	return nil
 }
 
 // read a file from a file system
 func read(filePath string) ([]byte, error) {
+	// #nosec G304 (file path provided as taint input)
+	// This line opens a file from the predefined storage.
+	// There is no user input.
 	readFile, err := os.Open(filePath)
 	if err != nil {
 		return nil, err
 	}
 
-	defer readFile.Close()
+	defer closeFile(readFile)
 
 	data, err := ioutil.ReadAll(readFile)
 	if err != nil {
@@ -152,6 +158,13 @@ func read(filePath string) ([]byte, error) {
 	return data, nil
 }
 
+func closeFile(file *os.File) {
+	err := file.Close()
+	if err != nil {
+		logger.Errorf("could not close file [%v]: [%v]", file.Name(), err)
+	}
+}
+
 // readAll reads all files from the provided directoryPath and outputs them
 // as DataDescriptors into the first returned output channel. All errors
 // occurred during file system reading are sent to the second output channel

tools/generators/ethereum/contract.go

@@ -61,6 +61,10 @@ func main() {
 	contractOutputPath := flag.Arg(1)
 	commandOutputPath := flag.Arg(2)
 
+	// #nosec G304 (file path provided as taint input)
+	// This line is placed in the auxiliary generator code,
+	// not in the core application. User input has to be passed to
+	// provide a path to the contract ABI.
 	abiFile, err := ioutil.ReadFile(abiPath)
 	if err != nil {
 		panic(fmt.Sprintf(
@@ -223,6 +227,12 @@ func organizeImports(outFile string, buf *bytes.Buffer) error {
 // Stores the Buffer `buf` content to a file in `filePath`
 func saveBufferToFile(buf *bytes.Buffer, filePath string) error {
 	file, err := os.Create(filePath)
+
+	// #nosec G104 G307 (audit errors not checked & deferring unsafe method)
+	// This line is placed in the auxiliary generator code,
+	// not in the core application. Also, the Close function returns only
+	// the error. It doesn't return any other values which can be a security
+	// threat when used without checking the error.
 	defer file.Close()
 	if err != nil {
 		return fmt.Errorf("output file %s creation failed [%v]", filePath, err)

tools/generators/promise/promise.go

@@ -96,12 +96,12 @@ func generatePromisesCode(generationDir string, promisesConfig []promiseConfig)
 		return fmt.Errorf("template creation failed [%v]", err)
 	}
 
-	for _, promiseConfig := range promisesConfig {
-		outputFile := promiseConfig.Filename
+	for i := range promisesConfig {
+		outputFile := promisesConfig[i].Filename
 		outputFilePath := path.Join(generationDir, outputFile)
 
 		// Generate promise code.
-		buffer, err := generateCode(promiseTemplate, &promiseConfig, outputFilePath)
+		buffer, err := generateCode(promiseTemplate, &promisesConfig[i], outputFilePath)
 		if err != nil {
 			return fmt.Errorf("promise generation failed [%v]", err)
 		}

tools/generators/template/template.go

@@ -30,6 +30,11 @@ func main() {
 	}
 
 	templateFile := os.Args[templateFileArgIndex]
+
+	// #nosec G304 (file path provided as taint input)
+	// This line is placed in the auxiliary generator code,
+	// not in the core application. User input has to be passed to provide a
+	// path to the template file.
 	templateContents, err := ioutil.ReadFile(templateFile)
 	if err != nil {
 		errorAndExit(fmt.Sprintf("Failed to open template file: [%v].", err))