How to Detect Unused Variables After Reassignment?
Hello everyone,
I’ve been working with Go and encountered a situation where I need to detect unused variables that have been reassigned. Specifically, I have code where an error variable ( err ) is reassigned and not checked after the reassignment. Here’s a simplified example:
In any case you would need to add a check (if) for this variable. For example add this lines to your code, (1) before the secod Atoi and after that Atoi.
fmt.Printf("%v:%v\n", &err, err)
In the first case as any error happened, err is nil. and in the second print your err has some value.:
That way you are sure it is a different variable from the previous one and it is only used in that scope, the narrow of teh scope, the better…
There are static code checkers which detect such, for example golangci-lint and staticcheck
@Helmut_Wais
Thank you! I think I’ll adopt it since it can be detected by golangci-lint.
Two linters I'll always add to new Go projects: errcheck and ineffassign
Jul 17, 2020
The reason for that is that they are the best bang for your buck by far. They are easy to appease, I have yet to see them produce false positives, they very frequently catch outright buggy code and retrofitting them is a pain.
ineffassign
This is the sort of thing that ineffassign prevents:
This is perfectly valid code, and will write garbage sometimes, without any indication whatsoever. Yes, Go does refuse to compile code like this
because data is not used, but declaring a variable and then immediately overwriting its value is perfectly legal and almost never what you want. At best the variable was never needed, in which case a _ is a good way to signal it, or it was meant to be used and ineffassign found a bug.
I’ve seen this pattern frequently enough in tests, where part of the test code accidentally doesn’t check intermediate errors or return values, leading to parts of the test silently breaking over time.
To its credit, gopls does check for this now .
In a similar vein, errcheck enforces checking error return values. In Go, errors are values, and unlike other 1 languages 2 , nothing enforces they are checked.
This is valid:
And so are all these:
The only difference is that the first example carries no indication where this was intentional or not. errcheck enforces that error values are at least assigned to _ , therefore being explicit that a decision was made to ignore the error 3 .
One case where this matters is when writing new code. It’s easy enough to forget about checking all error returns of all functions called. Again, tests passing by accident is a very frequent occasion, and so is production code.
Another also interesting case is functions changing signatures. When adding an error return to a function that previously returned nothing or updating a dependency that does so, you probably want to verify all the call sites, at the very least making the executive choice to explicitly ignore the errors.
Retrofitting using golangci-lint
golangci-lint has positioned itself as the tool everyone uses on CI, and I’d say with good reason. It supports many linters, has improved massively over the past couple of years and has facilities for wiring up into existing codebases by only checking code that changes 4 , allowing for incremental cleanup.
For example:
No one has to fix the unchecked error, until they touch the call in bar() . This works well, until you realise there are transformations where this heuristic falls flat. This is still true according to the latest golangci-lint, 1.28.3.
Here is an example of this in action:
Since the call to foo() is not touched, golangci-lint considers the unchecked error pre-existing and does not report it! The check is completely elided on changes that simply go from zero returns to a single error return. This simply makes the check not as useful as it could be, allowing regressions to merge over time.
The other problem with retrofitting is that the cleanup can be boring and take a long time. Clearing hundreds for errors in bulk is mind-numbing. Merely shifting around existing code might require fixing existing issues, unrelated to the change at hand.
Why go through that, when simply adding these linters from the start does the trick and saves you from bugs?
Addendum - 18th July
I got a bit curious about k8s’ code, and ran ineffassign against it. There is one case where ineffassign could be considered noisy, and that is using foo := true instead of var foo bool :
The code in question:
This nudges towards var exist bool or bool := false . Clearly there is no bug here, the result is the same either way, so it boils down to the style used when declaring variables.
good ↩
Rust ↩
Not necessarily a good decision, you can always find yourself staring at git blame wondering why. ↩
according to git and revgrep, using the new- settings in the config . Nowadays it works, a long time ago I found out the hard way it didn’t ↩
Trail of Bits Blog
Security assessment techniques for go projects.
- November 7, 2019
The Trail of Bits Assurance practice has received an influx of Go projects, following the success of our Kubernetes assessment this summer. As a result, we’ve been adapting for Go projects some of the security assessment techniques and tactics we’ve used with other compiled languages.
We started by understanding the design of the language, identifying areas where developers may not fully understand the functionality of a language semantic. Many of these misunderstood semantics originated from findings we reported to our clients and independent research into the language itself. While not exhaustive, some of these problem areas include scoping, coroutines, error handling, and dependency management. Notably, many of theses are not directly related to the runtime. The Go runtime itself is designed to be safe by default, preventing many C-like vulnerabilities.
With a better understanding of the root causes, we searched for existing tooling to help us quickly and effectively instrument client codebases. The result was a sample of static and dynamic open-source tools, including several that were Go-agnostic. To complement these tools, we also identified several compiler configurations that help with instrumentation.
Static analysis
Because Go is a compiled language, the compiler detects and prevents many potentially erroneous patterns before the binary executable is even produced. While this is a major annoyance for newer Go developers, these warnings are extremely important in preventing unexpected behavior and keeping code clean and readable.
Static analysis tends to catch a lot of very low hanging fruit not included in compiler errors and warnings. Within the Go ecosystem, there are many disparate tools such as go-vet , staticcheck , and those within the analysis package. These tools typically identify problems like variable shadowing, unsafe pointer use, and unused function return values. Investigating the areas of a project where these tools display warnings typically leads to exploitable functionality.
These tools are by no means perfect. For example, go-vet can miss very common accidents like the one below, where the A function’s err return value is unused, and immediately reassigned during the assignment of bSuccess on the left-hand side of the expression. The compiler will not provide a warning, and go-vet does not detect this; nor does errcheck . In fact, the tools that successfully identify this case (non-exhaustive) are the aforementioned staticcheck and ineffassign , which identify the err return value of A as unused or ineffectual.
Figure 1: An example program showing an ineffectual assignment of err tricking go-vet and errcheck into considering err as checked.
Figure 2: The output of the example program, along with errcheck, go-vet, staticcheck, and ineffassign.
When you look deeper into this example, you may wonder why the compiler does not warn on this problem. The Go compiler will error when variables are not used within a program, but this example successfully compiles. This is caused by the semantics of the “short variable declaration.”
Figure 3: The grammar specification of the “short variable declaration.”
According to the specification, the short variable declaration has the special ability to redeclare variables as long as:
- The redeclaration is in a multi-variable short declaration.
- The redeclared variable is declared earlier in the same block or function’s parameter list.
- The redeclared variable is of the same type as the previous declaration.
- At least one non-blank variable in the declaration is new.
All of these constraints hold in the previous example, preventing the compiler from producing errors for this problem.
Many tools have edge cases like this where they are unsuccessful in identifying related issues, or identify an issue but describe it differently. Compounding the problem, these tools often require building the Go source code before analysis can be performed. This makes third-party security assessments complicated if the analysts cannot easily build the codebase or its dependencies.
Despite these pitfalls, when put together, the available tools can provide good hints as to where to look for problems within a given project, with just a little bit of effort. We recommend using gosec , go-vet , and staticcheck , at a minimum. These have the best documentation and ergonomics for most codebases. They also provide a wide variety of checks (such as ineffassign or errcheck ) for common issues, without getting too specific. For more in-depth analysis of a particular type of issue, however, one might have to use the more specific analyzers, develop custom tooling directly against the SSA , or use $emmle .
Dynamic analysis
Once static analysis has been performed and the results have been reviewed, dynamic analysis techniques are typically the next step for deeper results. Due to Go’s memory safety, the problems normally found with dynamic analysis are those that result in a hard crash or an invalidation of program state. Various tools and approaches have been built to help identify these types of issues within the Go ecosystem. Additionally, it’s possible to retrofit existing language-agnostic tooling for the dynamic testing of Go software, which we show next.
The best-known dynamic testing tool in the Go space is likely Dimitry Vyukov’s implementation of dvyukov/go-fuzz . This tool allows you to quickly and effectively implement mutational fuzzing. It even has an extensive wall of trophies . More advanced users may also find the distributed fuzzing and libFuzzer support useful when hunting for bugs.
Google also produced a more primitive fuzzer with a confusingly similar name, google/gofuzz , that assists users by initializing structures with random values. Unlike Dimitry’s go-fuzz , Google’s gofuzz does not generate a harness or assist with storing crash output, fuzzed input, or any other type of information. While this can be a downside for testing some targets, it makes for a lightweight and extensible framework.
For the sake of brevity, we refer you to examples of both tools in their respective READMEs.
- google/gofuzz#gofuzz
- dvyukov/go-fuzz#usage
Property testing
Diverging from more traditional fuzzing approaches, Go’s testing package (typically used for unit and integration testing) provides the testing/quick sub-package for “black box testing” of Go functions. In other terms, it is a basic primitive for property testing. Given a function and generator, the package can be used to build a harness to test for potential property violations given the range of the input generator. The following example is pulled directly from the documentation .
Figure 4: The OddMultipleOfThree function is being tested, where its return value should always be an odd multiple of three. If it’s not, the f function will return false and the property will be violated. This is detected by the quick. Check function.
While the functionality provided by this package is acceptable for simple applications of property testing, important properties do not often fit well into such a basic interface. To address these shortcomings, the leanovate/gopter framework was born. Gopter provides a wide variety of generators for the common Go types, and has helpers to assist you in creating your own generators compatible with Gopter. Stateful tests are also supported through the gopter/commands sub-package, which is useful for testing that properties hold across sequences of actions. Compounding this, when a property is violated, Gopter shrinks the generated inputs. See a brief example of property tests with input shrinking in the output below.
Figure 5: The testing harness for the Compute structure.
Figure 6: Executing the test harness and observing the output of the property tests, where Divide fails.
Fault injection
Fault injection has been surprisingly effective when attacking Go systems. The most common mistakes we found using this method involve the handling of the error type. Since error is only a type in Go, when it is returned it does not change a program’s execution flow on it’s own like a panic statement would. We identify such bugs by enforcing errors from the lowest level: the kernel. Because Go produces static binaries, faults must be injected without LD_PRELOAD . One of our tools, KRF , allows us to do exactly this.
During our recent assessment of the Kubernetes codebase, the use of KRF provided a finding deep inside a vendored dependency, simply by randomly faulting read and write system calls spawned by a process and its children. This technique was effective against the Kubelet, which commonly interfaces with the underlying system. The bug was triggered when the ionice command was faulted, producing no output to STDOUT and sending an error to STDERR . After the error was logged, execution continued instead of returning the error in STDERR to the caller. This results in STDOUT later being indexed, causing an index out of range runtime panic.
Figure 7: The shortened callstack of the resulting Kubelet panic.
Figure 8: The logging of STDERR without returning the error to the caller.
Figure 9: The attempted indexing of STDOUT, even though it is empty. This is the cause of the runtime panic.
For a more complete walkthrough containing reproduction steps, our Kubernetes Final Report details the use of KRF against the Kubelet in Appendix G (pg. 109).
Go’s compiler also allows instrumentation to be included in a binary, which permits detection of race conditions at runtime. This is extremely useful for identifying potentially exploitable races as an attacker, but it can also be leveraged to identify incorrect handling of defer, panic, and recover . We built trailofbits/on-edge to do exactly this: Identify global state changes between a function entrypoint and the point at which a function panics, and exfiltrate this information through the Go race detector. More in-depth use of OnEdge can be found in our previous blog post, “Panicking the Right Way in Go.”
In practice, we recommend using:
- dvyukov/go-fuzz to build harnesses for components parsing input,
- google/gofuzz for testing structure validations,
- leanovate/gopter for augmenting existing unit and integration tests and testing specification correctness, and
- trailofbits/krf and trailofbits/on-edge for testing error handling.
All of these tools, with the exception of KRF, require a bit of effort to use in practice.
Using the compiler to our advantage
The Go compiler has many built-in features and directives that aid in finding bugs. These features are hidden in and throughout various switches, and require a bit of configuration for our purposes.
Subverting the type system
Sometimes when attempting to test the functionality of a system, the exported functions aren’t what we want to test. Getting testable access to the desired functions may require renaming a lot of them so they can be exported, which can be burdensome. To help address this problem, the build directives of the compiler can be used to perform name linking, accessing controls provided by the export system. As an example of this functionality, the program below (graciously extracted from a Stack Overflow answer ) accesses the unexported reflect.typelinks function and subsequently iterates the type link table to identify types present in the compiled program.
Figure 10: A generalized version of the Stack Overflow answer, using the link name build directive.
Figure 11: The output of the typelinks table.
In situations where you need even more granular control at runtime (i.e., more than just the link name directive), you can write in Go’s intermediate assembly and include it during compilation. While it may be incomplete and slightly out of date in some places, the teh-cmc/go-internals repository provides a great introduction to how Go assembles functions.
Compiler-generated coverage maps
To help with testing, the Go compiler can perform preprocessing to generate coverage information . This is intended for identifying unit and integration testing coverage information, but we can also use it to identify coverage generated by our fuzzing and property testing. Filippo Valsorda provides a simple example of this in a blog post .
Type-width safety
Go has support for automatically determining the size of integers and floating-point numbers based on the target platform. However, it also allows for fixed-width definitions, such as int32 and int64 . When mixing both automatic and fixed-width sizes, there are opportunities for incorrect assumptions about behavior across multiple target platforms.
Testing against both 32-bit and 64-bit platform builds of a target will help identify platform-specific problems. These problems tend to be found in areas performing validation, decoding, or type conversion, where improper assumptions about the source and destination type properties are made. Examples of this were identified in the Kubernetes security assessment, specifically TOB-K8S-015: Overflows when using strconv.Atoi and downcasting the result (pg. 42 in the Kubernetes Final Report ), with an example inlined below.
Figure 12: An example of downcasting to a fixed-width integer from an automatic-width integer (returned by Atoi).
Figure 13: The resulting overflow from incorrect type-width assumptions.
In practice, the type system subversion is rarely necessary. The most interesting targets for testing are already exported, available through traditional imports. We recommend using this only when helpers and similar unexported functions are required for testing. As for testing type-width safety, we recommend compiling against all targets when possible, even if it is not directly supported, since problems may be more apparent on different targets. Finally, we recommend generating coverage reports on projects with unit and integration tests, at a minimum. It helps identify areas that are not directly tested, which can be prioritized for review.
A note about dependencies
In languages such as JavaScript and Rust , dependency managers have built-in support for dependency auditing—scanning project dependencies for versions known to have vulnerabilities. In Go, no such tool exists, at least not in a publicly available and non-experimental state.
This lack likely stems from the fact that there are many different methods of dependency management: go-mod , go-get , vendored , etc. These various methods use radically different approaches, resulting in no straightforward way to universally identify dependencies and their versions. Furthermore, in some cases it is common for developers to subsequently modify their vendored dependency source code.
The problem of dependency management has progressed over the years of Go’s development, and most developers are moving towards the use of go mod . This allows dependencies to be tracked and versioned within a project through the go.mod file , opening the door for future dependency scanning efforts. An example of such an effort can be seen within the OWASP DependencyCheck tool, which has an experimental go mod plugin .
Ultimately, there are quite a few tools available for use within the Go ecosystem. Although mostly disparate, the various static analysis tools help identify “low hanging fruit” within a given project. When looking for deeper concerns, fuzzing, property testing, and fault injection tools are readily available. Compiler configuration subsequently augments the dynamic techniques, making it easier to build harnesses and evaluate their effectiveness.
Interested in seeing these techniques shake out bugs in your Go systems? Trail of Bits can make that happen. Do you want custom analysis built specifically for your organization? We do that too. Contact us !
Share this:
5 thoughts on “ security assessment techniques for go projects ”.
Pingback: Security assessment techniques for Go projects – Hacker News Robot
Excellent post thanks a lot for sharing this, secure development practices are very important in product development. Wanted to give a shout-out to https://github.com/golangci/golangci-lint for bringing some of the linters mentioned in the post into a nice bundle.
Pingback: Security assessment techniques for Go projects - Golang News
Actually, Snyk is free and the CLI is open source tool for security vulnerabilities scanning and it supports Go projects that are managed with dep, govendor or go modules. How to get started here: https://snyk.io/docs/snyk-for-golang/ and there’s a whole vulnerabilities database open to browsing at https://snyk.io/vuln?type=golang for Golang.
Full disclosure: I’m a developer advocate at Snyk
CodeQL is a tool for static analysis that supports Go
https://help.semmle.com/QL/learn-ql/go/ql-for-go.html
It’s free for open-source projects and I think they offer options to use it for proprietary software. In fact, the rules for detecting issues are open-sourced. Anybody can contribute.
Leave a Reply Cancel reply
Discover more from trail of bits blog.
Subscribe now to keep reading and get access to the full archive.
Type your email…
Continue reading
| |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
IMAGES
VIDEO
COMMENTS
Jun 23, 2021 at 5:25. 2. @Ishmeet, note that the linter wasn't forcing you to use a var declaration; instead it hinted at that you might have a bug in your code—because the first assignment could be way more involved like assigning the return value of some function call which would return a value inintialized in a complicated way, and ...
There are static code checkers which detect such, for example golangci-lint and staticcheck $ golangci-lint run x.go:15:8: ineffectual assignment to err (ineffassign) num2, err := strconv.Atoi("a") $ $ staticcheck ./... x.go:15:2: this value of err is never used (SA4006)
ineffassign. Detect ineffectual assignments in Go code. An assignment is ineffectual if the variable assigned is not thereafter used. This tool misses some cases because it does not consider any type information in its analysis. For example, assignments to struct fields are never marked as ineffectual. It should, however, never give any false ...
ineffassign. Detect ineffectual assignments in Go code. An assignment is ineffectual if the variable assigned is not thereafter used. This tool misses some cases because it does not consider any type information in its analysis. For example, assignments to struct fields are never marked as ineffectual. It should, however, never give any false ...
ineffassign version: last version available go version: go version go1.18.3 Running the linter when interacting with a var after passing it to a deferred function call marks it as ineffectual assignment package main import "errors" func ...
[BUG][GO] ineffectual assignment to err in decode #10064. Closed 5 of 6 tasks. brianlow opened this issue Jul 30, 2021 · 2 comments · Fixed by #10275. Closed 5 of 6 tasks [BUG][GO] ineffectual assignment to err in decode #10064. brianlow opened this issue Jul 30, 2021 · 2 comments · Fixed by #10275. Labels.
The reason for that is that they are the best bang for your buck by far. They are easy to appease, I have yet to see them produce false positives, they very frequently catch outright buggy code and retrofitting them is a pain.
Figure 1: An example program showing an ineffectual assignment of err tricking go-vet and errcheck into considering err as checked. $ go run . false : true $ errcheck . ... In situations where you need even more granular control at runtime (i.e., more than just the link name directive), ...
Fortunately there's a linter called ineffassign that tracks ineffective assignments, and it's part of gometalinter and golangci-lint. But not "go vet". And there's no linter that can detect the first example that I gave. Shadowing is a pet peeve of mine. Go is one of the strictest, most opinionated languages in mainstream use, but it's ...
Hi, if it's possible, I would like to work on this. I had run ineffassign on x/text again an found more results. See the the updated list below. Almost all issues can be easily fixed by simply removing or refactoring the ineffectual assignments clauses.
$ golangci-lint run ./api/ --disable-all-E deadcode -E gosimple -E gosec -E govet -E ineffassign -E staticcheck -E structcheck -E typecheck -E unused -E varcheck comment_get.go:36: G202: ... (url) oauth_github_callback.go:12:8: ineffectual assignment to ` err ` (ineffassign) resp, err : = http.Get ...
I'm glad you find it funny! The backstory is that I actually wrote this whole blog series criticising Go a few weeks ago, and then by the end of it felt like it was a little too much negativity to just go and post online.
Instructor resource. Details. APA Style Refresher for Instructors: Reinforcing the Basics and Avoiding Ghost and Zombie Guidelines. Recording of a webinar conducted in October 2023 to refresh educators' understanding of the basics of APA Style, help them avoid outdated APA Style guidelines ("zombie guidelines"), debunk APA Style myths ("ghost guidelines"), and help students learn APA ...
Detect ineffectual assignments in Go code. This tool misses some cases because does not consider any type information in its analysis. (For example, assignments to struct fields are never marked as ineffectual.) It should, however, never give any false positives.
ineffectual assignment to err #128. Open tegk opened this issue Nov 7, 2019 · 2 comments Open ineffectual assignment to err #128. tegk opened this issue Nov 7, 2019 · 2 ... I generally suggest to not use naked returns as assignment bugs can be prevented by this. Something like this should be much better to read :-) func (s *Server ...
ineffassign. Detect ineffectual assignments in Go code. This tool misses some cases because does not consider any type information in its analysis. (For example, assignments to struct fields are never marked as ineffectual.) It should, however, never give any false positives.
The injured Minnesota Twins center fielder is closing in on the start of a rehab assignment, though he has a few more steps to take. Out since Aug. 15 with right hip inflammation, Buxton reported ...
Angels pitching prospect Chase Silseth underwent elbow surgery earlier this month and will miss the rest of the season. He did not undergo Tommy John surgery, and is expected to be ready to return ...
package main import "fmt" func main() { var i int i = 3 i = 5 fmt.Println(i) } $ ineffassign main.go does not produce any results. It should catch that the i = 3 assignment was ineffective (its value was never used). I created the issue ...
4. This is because in the second case you are re-using an existing err variable, so it is being used. Despite the := instantiate & assign operator, a new err variable is not instantiated. If you named the errors differently, such as this: func main() {. a, err := strconv.Atoi("100") if err != nil {. panic(err)
Thousands of students across the country will soon be finding out their GCSE results and thinking about the next steps in their education.. Here we explain everything you need to know about the big day, from when results day is, to the current 9-1 grading scale, to what your options are if your results aren't what you're expecting.
ineffectual assignment to err #338. Closed tegk opened this issue Jun 12, 2019 · 1 comment Closed ineffectual assignment to err #338. tegk opened this issue Jun 12, 2019 · 1 comment Comments. Copy link tegk commented Jun 12, 2019. We should do something with err :-)
Saved searches Use saved searches to filter your results more quickly