gosec icon indicating copy to clipboard operation
gosec copied to clipboard

G304 on `os.Executable`

Open asiffer opened this issue 3 years ago • 1 comments

Summary

G304 is raised when we try to read the output of os.Executable()

Steps to reproduce the behavior

package main

import (
	"fmt"
	"io/ioutil"
	"os"
	"path/filepath"
)

func main() {
	bin, err := os.Executable()
	if err != nil {
		panic(err)
	}
	bin = filepath.Clean(bin)

	// https://pkg.go.dev/path/filepath#EvalSymlinks
	// "EvalSymlinks calls Clean on the result"
	bin, err = filepath.EvalSymlinks(bin)
	if err != nil {
		panic(err)
	}

	raw, err := ioutil.ReadFile(bin)
	if err != nil {
		panic(err)
	}

	fmt.Println(raw[:32])
}
gosec .

gosec version

Version: 2.11.0
Git tag: v2.11.0
Build date: 2022-03-21T15:55:53Z

Go version (output of 'go version')

go version go1.18.3 linux/amd64

Operating system / Environment

Expected behavior

I not 100% sure that no error should be raised. If it remains dangerous, having a way to fix this would be great.

Actual behavior

G304 is raised.

[./main.go:24] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    23: 
  > 24:         raw, err := ioutil.ReadFile(bin)
    25:         if err != nil {

asiffer avatar Jun 08 '22 08:06 asiffer

@asiffer

There is no way to make it safe(as far as I can understand). os.Executables might return symlink. In case If since the process was started the symlink was changed, it would return "tainted" path. Also, the EvalSymLink function won't help here because it's just evals symlink and cleans the path to the result. It doesn't mean that you'll get the result you expected(path to your executable). It can be anything.

timonomsk avatar Aug 07 '22 23:08 timonomsk

This is fixed by https://github.com/securego/gosec/pull/912.

ccojocar avatar Jan 09 '23 16:01 ccojocar