diff --git a/change-notes/2020-04-30-syscall-functions.md b/change-notes/2020-04-30-syscall-functions.md new file mode 100644 index 00000000000..caa2cf56654 --- /dev/null +++ b/change-notes/2020-04-30-syscall-functions.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* The query "Command built from user-controlled sources" has been improved to recognize methods from the `syscall` library, which may lead to more alerts. diff --git a/change-notes/2020-05-20-request-forgery-sanitizers.md b/change-notes/2020-05-20-request-forgery-sanitizers.md new file mode 100644 index 00000000000..cb1fdcb5acf --- /dev/null +++ b/change-notes/2020-05-20-request-forgery-sanitizers.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* The query "Uncontrolled data used in network request" is now more precise, which may reduce the number of false positives. diff --git a/change-notes/2020-06-19-switch-block-without-test.md b/change-notes/2020-06-19-switch-block-without-test.md new file mode 100644 index 00000000000..17f20f6840f --- /dev/null +++ b/change-notes/2020-06-19-switch-block-without-test.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* A bug has been fixed that could cause the incorrect analysis of control flow around switch statements. diff --git a/change-notes/2020-06-26-taint-model-tar-zip.md b/change-notes/2020-06-26-taint-model-tar-zip.md new file mode 100644 index 00000000000..10cdaa41b46 --- /dev/null +++ b/change-notes/2020-06-26-taint-model-tar-zip.md @@ -0,0 +1,3 @@ +lgtm,codescanning +* Modeling of the `archive/tar` and `archive/zip` packages has been added, which may lead to more + results from the security queries. diff --git a/change-notes/2020-07-06-repo-with-file-url-origin.md b/change-notes/2020-07-06-repo-with-file-url-origin.md new file mode 100644 index 00000000000..f84d4c995a2 --- /dev/null +++ b/change-notes/2020-07-06-repo-with-file-url-origin.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* A bug has been fixed that caused the autobuilder to not work on repositories with a `file://` URL as `origin`. diff --git a/extractor/cli/go-autobuilder/go-autobuilder.go b/extractor/cli/go-autobuilder/go-autobuilder.go index 4df3d2c77e0..63d8aa153d8 100644 --- a/extractor/cli/go-autobuilder/go-autobuilder.go +++ b/extractor/cli/go-autobuilder/go-autobuilder.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "golang.org/x/mod/semver" "io/ioutil" "log" "net/url" @@ -44,12 +45,27 @@ variable is 32. fmt.Fprintf(os.Stderr, "Usage:\n\n %s\n", os.Args[0]) } +var goVersion = "" + +// Returns the current Go version as returned by 'go version', e.g. go1.14.4 func getEnvGoVersion() string { - gover, err := exec.Command("go", "version").CombinedOutput() - if err != nil { - log.Fatalf("Unable to run the go command, is it installed?\nError: %s", err.Error()) + if goVersion == "" { + gover, err := exec.Command("go", "version").CombinedOutput() + if err != nil { + log.Fatalf("Unable to run the go command, is it installed?\nError: %s", err.Error()) + } + goVersion = strings.Fields(string(gover))[2] } - return strings.Fields(string(gover))[2] + return goVersion +} + +// Returns the current Go version in semver format, e.g. v1.14.4 +func getEnvGoSemVer() string { + goVersion := getEnvGoVersion() + if !strings.HasPrefix(goVersion, "go") { + log.Fatalf("Expected 'go version' output of the form 'go1.2.3'; got '%s'", goVersion) + } + return "v" + goVersion[2:] } func run(cmd *exec.Cmd) bool { @@ -141,6 +157,55 @@ const ( Glide ) +// ModMode corresponds to the possible values of the -mod flag for the Go compiler +type ModMode int + +const ( + ModUnset ModMode = iota + ModReadonly + ModMod + ModVendor +) + +func (m ModMode) argsForGoVersion(version string) []string { + switch m { + case ModUnset: + return []string{} + case ModReadonly: + return []string{"-mod=readonly"} + case ModMod: + if !semver.IsValid(version) { + log.Fatalf("Invalid Go semver: '%s'", version) + } + if semver.Compare(version, "v1.14") < 0 { + return []string{} // -mod=mod is the default behaviour for go <= 1.13, and is not accepted as an argument + } else { + return []string{"-mod=mod"} + } + case ModVendor: + return []string{"-mod=vendor"} + } + return nil +} + +// addVersionToMod add a go version directive, e.g. `go 1.14` to a `go.mod` file. +func addVersionToMod(goMod []byte, version string) bool { + cmd := exec.Command("go", "mod", "edit", "-go="+version) + return run(cmd) +} + +// checkVendor tests to see whether a vendor directory is inconsistent according to the go frontend +func checkVendor() bool { + vendorCheckCmd := exec.Command("go", "list", "-mod=vendor", "./...") + outp, err := vendorCheckCmd.CombinedOutput() + if err != nil { + badVendorRe := regexp.MustCompile(`(?m)^go: inconsistent vendoring in .*:$`) + return !badVendorRe.Match(outp) + } + + return true +} + func main() { if len(os.Args) > 1 { usage() @@ -168,6 +233,7 @@ func main() { // determine how to install dependencies and whether a GOPATH needs to be set up before // extraction depMode := GoGetNoModules + modMode := ModUnset needGopath := true if util.FileExists("go.mod") { depMode = GoGetWithModules @@ -183,7 +249,40 @@ func main() { // if a vendor/modules.txt file exists, we assume that there are vendored Go dependencies, and // skip the dependency installation step and run the extractor with `-mod=vendor` - hasVendor := util.FileExists("vendor/modules.txt") + if util.FileExists("vendor/modules.txt") { + modMode = ModVendor + } else if util.DirExists("vendor") { + modMode = ModMod + } + + if modMode == ModVendor { + // fix go vendor issues with go versions >= 1.14 when no go version is specified in the go.mod + // if this is the case, and dependencies were vendored with an old go version (and therefore + // do not contain a '## explicit' annotation, the go command will fail and refuse to do any + // work + // + // we work around this by adding an explicit go version of 1.13, which is the last version + // where this is not an issue + if depMode == GoGetWithModules { + goMod, err := ioutil.ReadFile("go.mod") + if err != nil { + log.Println("Failed to read go.mod to check for missing Go version") + } else if versionRe := regexp.MustCompile(`(?m)^go[ \t\r]+[0-9]+\.[0-9]+$`); !versionRe.Match(goMod) { + // if the go.mod does not contain a version line + modulesTxt, err := ioutil.ReadFile("vendor/modules.txt") + if err != nil { + log.Println("Failed to read vendor/modules.txt to check for mismatched Go version") + } else if explicitRe := regexp.MustCompile("(?m)^## explicit$"); !explicitRe.Match(modulesTxt) { + // and the modules.txt does not contain an explicit annotation + log.Println("Adding a version directive to the go.mod file as the modules.txt does not have explicit annotations") + if !addVersionToMod(goMod, "1.13") { + log.Println("Failed to add a version to the go.mod file to fix explicitly required package bug; not using vendored dependencies") + modMode = ModMod + } + } + } + } + } // if `LGTM_INDEX_NEED_GOPATH` is set, it overrides the value for `needGopath` inferred above if needGopathOverride := os.Getenv("LGTM_INDEX_NEED_GOPATH"); needGopathOverride != "" { @@ -291,7 +390,7 @@ func main() { // check whether an explicit dependency installation command was provided inst := util.Getenv("CODEQL_EXTRACTOR_GO_BUILD_COMMAND", "LGTM_INDEX_BUILD_COMMAND") - var install *exec.Cmd + shouldInstallDependencies := false if inst == "" { // if there is a build file, run the corresponding build tool buildSucceeded := tryBuild("Makefile", "make") || @@ -302,54 +401,8 @@ func main() { tryBuild("build.sh", "./build.sh") if !buildSucceeded { - if hasVendor { - log.Printf("Skipping dependency installation because a Go vendor directory was found.") - } else { - // automatically determine command to install dependencies - if depMode == Dep { - // set up the dep cache if SEMMLE_CACHE is set - cacheDir := os.Getenv("SEMMLE_CACHE") - if cacheDir != "" { - depCacheDir := filepath.Join(cacheDir, "go", "dep") - log.Printf("Attempting to create dep cache dir %s\n", depCacheDir) - err := os.MkdirAll(depCacheDir, 0755) - if err != nil { - log.Printf("Failed to create dep cache directory: %s\n", err.Error()) - } else { - log.Printf("Setting dep cache directory to %s\n", depCacheDir) - err = os.Setenv("DEPCACHEDIR", depCacheDir) - if err != nil { - log.Println("Failed to set dep cache directory") - } else { - err = os.Setenv("DEPCACHEAGE", "720h") // 30 days - if err != nil { - log.Println("Failed to set dep cache age") - } - } - } - } - - if util.FileExists("Gopkg.lock") { - // if Gopkg.lock exists, don't update it and only vendor dependencies - install = exec.Command("dep", "ensure", "-v", "-vendor-only") - } else { - install = exec.Command("dep", "ensure", "-v") - } - log.Println("Installing dependencies using `dep ensure`.") - } else if depMode == Glide { - install = exec.Command("glide", "install") - log.Println("Installing dependencies using `glide install`") - } else { - if depMode == GoGetWithModules { - // enable go modules if used - os.Setenv("GO111MODULE", "on") - } - - // get dependencies - install = exec.Command("go", "get", "-v", "./...") - log.Println("Installing dependencies using `go get -v ./...`.") - } - } + // Build failed; we'll try to install dependencies ourselves + shouldInstallDependencies = true } } else { // write custom build commands into a script, then run it @@ -380,12 +433,70 @@ func main() { log.Fatalf("Unable to close temporary script holding custom build commands: %s\n", err.Error()) } os.Chmod(script.Name(), 0700) - install = exec.Command(script.Name()) log.Println("Installing dependencies using custom build command.") + run(exec.Command(script.Name())) } - if install != nil { - run(install) + if modMode == ModVendor { + // test if running `go` with -mod=vendor works, and if it doesn't, try to fallback to -mod=mod + // or not set if the go version < 1.14. Note we check this post-build in case the build brings + // the vendor directory up to date. + if !checkVendor() { + modMode = ModMod + log.Println("The vendor directory is not consistent with the go.mod; not using vendored dependencies.") + } + } + + if shouldInstallDependencies { + if modMode == ModVendor { + log.Printf("Skipping dependency installation because a Go vendor directory was found.") + } else { + // automatically determine command to install dependencies + var install *exec.Cmd + if depMode == Dep { + // set up the dep cache if SEMMLE_CACHE is set + cacheDir := os.Getenv("SEMMLE_CACHE") + if cacheDir != "" { + depCacheDir := filepath.Join(cacheDir, "go", "dep") + log.Printf("Attempting to create dep cache dir %s\n", depCacheDir) + err := os.MkdirAll(depCacheDir, 0755) + if err != nil { + log.Printf("Failed to create dep cache directory: %s\n", err.Error()) + } else { + log.Printf("Setting dep cache directory to %s\n", depCacheDir) + err = os.Setenv("DEPCACHEDIR", depCacheDir) + if err != nil { + log.Println("Failed to set dep cache directory") + } else { + err = os.Setenv("DEPCACHEAGE", "720h") // 30 days + if err != nil { + log.Println("Failed to set dep cache age") + } + } + } + } + + if util.FileExists("Gopkg.lock") { + // if Gopkg.lock exists, don't update it and only vendor dependencies + install = exec.Command("dep", "ensure", "-v", "-vendor-only") + } else { + install = exec.Command("dep", "ensure", "-v") + } + log.Println("Installing dependencies using `dep ensure`.") + } else if depMode == Glide { + install = exec.Command("glide", "install") + log.Println("Installing dependencies using `glide install`") + } else { + if depMode == GoGetWithModules { + // enable go modules if used + os.Setenv("GO111MODULE", "on") + } + // get dependencies + install = exec.Command("go", "get", "-v", "./...") + log.Println("Installing dependencies using `go get -v ./...`.") + } + run(install) + } } // extract @@ -403,15 +514,14 @@ func main() { log.Fatalf("Unable to determine current directory: %s\n", err.Error()) } - var cmd *exec.Cmd - // check for `vendor/modules.txt` and not just `vendor` in order to distinguish non-go vendor dirs - if depMode == GoGetWithModules && hasVendor { - log.Printf("Running extractor command '%s -mod=vendor ./...' from directory '%s'.\n", extractor, cwd) - cmd = exec.Command(extractor, "-mod=vendor", "./...") - } else { - log.Printf("Running extractor command '%s ./...' from directory '%s'.\n", extractor, cwd) - cmd = exec.Command(extractor, "./...") + extractorArgs := []string{} + if depMode == GoGetWithModules { + extractorArgs = append(extractorArgs, modMode.argsForGoVersion(getEnvGoSemVer())...) } + extractorArgs = append(extractorArgs, "./...") + + log.Printf("Running extractor command '%s %v' from directory '%s'.\n", extractor, extractorArgs, cwd) + cmd := exec.Command(extractor, extractorArgs...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr err = cmd.Run() diff --git a/extractor/util/util.go b/extractor/util/util.go index c8eeb33f484..a592224672c 100644 --- a/extractor/util/util.go +++ b/extractor/util/util.go @@ -91,3 +91,12 @@ func FileExists(filename string) bool { } return err == nil && !info.IsDir() } + +// DirExists tests whether `filename` exists and is a directory. +func DirExists(filename string) bool { + info, err := os.Stat(filename) + if err != nil && !os.IsNotExist(err) { + log.Printf("Unable to stat %s: %s\n", filename, err.Error()) + } + return err == nil && info.IsDir() +} diff --git a/ql/src/semmle/go/StringOps.qll b/ql/src/semmle/go/StringOps.qll index dfc8ceb3c9f..f844d945072 100644 --- a/ql/src/semmle/go/StringOps.qll +++ b/ql/src/semmle/go/StringOps.qll @@ -67,7 +67,7 @@ module StringOps { } /** - * An expression of form `strings.HasPrefix(A, B)`. + * An expression of the form `strings.HasPrefix(A, B)`. */ private class StringsHasPrefix extends Range, DataFlow::CallNode { StringsHasPrefix() { getTarget().hasQualifiedName("strings", "HasPrefix") } @@ -78,15 +78,25 @@ module StringOps { } /** - * An expression of form `strings.Index(A, B) === 0`. + * Holds if `eq` is of the form `nd == 0` or `nd != 0`. + */ + pragma[noinline] + private predicate comparesToZero(DataFlow::EqualityTestNode eq, DataFlow::Node nd) { + exists(DataFlow::Node zero | + eq.hasOperands(globalValueNumber(nd).getANode(), zero) and + zero.getIntValue() = 0 + ) + } + + /** + * An expression of the form `strings.Index(A, B) == 0`. */ private class HasPrefix_IndexOfEquals extends Range, DataFlow::EqualityTestNode { DataFlow::CallNode indexOf; HasPrefix_IndexOfEquals() { - indexOf.getTarget().hasQualifiedName("strings", "Index") and - getAnOperand() = globalValueNumber(indexOf).getANode() and - getAnOperand().getIntValue() = 0 + comparesToZero(this, indexOf) and + indexOf.getTarget().hasQualifiedName("strings", "Index") } override DataFlow::Node getBaseString() { result = indexOf.getArgument(0) } @@ -97,19 +107,30 @@ module StringOps { } /** - * A comparison of form `x[0] === 'k'` for some rune literal `k`. + * Holds if `eq` is of the form `str[0] == rhs` or `str[0] != rhs`. + */ + pragma[noinline] + private predicate comparesFirstCharacter( + DataFlow::EqualityTestNode eq, DataFlow::Node str, DataFlow::Node rhs + ) { + exists(DataFlow::ElementReadNode read | + eq.hasOperands(globalValueNumber(read).getANode(), rhs) and + str = read.getBase() and + str.getType().getUnderlyingType() instanceof StringType and + read.getIndex().getIntValue() = 0 + ) + } + + /** + * A comparison of the form `x[0] == 'k'` for some rune literal `k`. */ private class HasPrefix_FirstCharacter extends Range, DataFlow::EqualityTestNode { - DataFlow::ElementReadNode read; + DataFlow::Node base; DataFlow::Node runeLiteral; - HasPrefix_FirstCharacter() { - read.getBase().getType().getUnderlyingType() instanceof StringType and - read.getIndex().getIntValue() = 0 and - eq(_, globalValueNumber(read).getANode(), runeLiteral) - } + HasPrefix_FirstCharacter() { comparesFirstCharacter(this, base, runeLiteral) } - override DataFlow::Node getBaseString() { result = read.getBase() } + override DataFlow::Node getBaseString() { result = base } override DataFlow::Node getSubstring() { result = runeLiteral } @@ -117,7 +138,7 @@ module StringOps { } /** - * A comparison of form `x[:len(y)] === y`. + * A comparison of the form `x[:len(y)] == y`. */ private class HasPrefix_Substring extends Range, DataFlow::EqualityTestNode { DataFlow::SliceNode slice; diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll index b1f28852e14..5fdd170574e 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll @@ -672,6 +672,13 @@ class BinaryOperationNode extends Node { /** Gets the operator of this operation. */ string getOperator() { result = op } + + /** Holds if `x` and `y` are the operands of this operation, in either order. */ + predicate hasOperands(Node x, Node y) { + x = getAnOperand() and + y = getAnOperand() and + x != y + } } /**