GOLANG CODE REVIEW NOTES

来源:岁月联盟 编辑:猪蛋儿 时间:2022-07-04

INTRO

Our aim is both to collate various resources and patterns one can find on the Internet, but also to add a couple of notes about quirks of the language that could lead to security vulnerabilities.

PREFACE

Go in the past years has become a language of many faces. It’s compiled and garbage collected nature with easy cross-compilation support for many architectures make it ideal for embedded and IoT projects, while the simplicity of goroutines make it ideal for web-based projects, which is further supported by the many third-party libraries. Not to mention its built-in testing framework which now natively supports fuzzing.

Because of the popularity of the language, in this blog post we wanted to highlight some code constructs for security engineers to look out for.

A quick note on Golang versions: the examples in this post were tested on Go v1.17 and v1.18. Future changes to the language might alter the described behaviour.

GETTING STARTED

Before we get to the juicy bits though, I would like to take a moment to highlight a couple of tools that come really handy when one is facing a dauntingly large code base. To get our bearings, of course we could start with the route handlers in a web project or a function dispatcher in a command line tool and go from there. However, if we want to get a quick overview of some of the hotspots, files where complexity is high, we have two tools that can come to our aid:

  • abcgo, which will measure the ABC metrics of each file
  • gocyclo can help us get a feel for each file’s cyclomatic complexity

With the information from these two tools and our preferred code review approach we can quickly home in on parts of the code base where some juicy bugs might reside.

BUG CLASSES

Even though Go is a memory managed language with a strong focus on security be default, there are still some dangerous code constructs developers need to keep an eye on. The goal of this blog post is not to list each of the relevant bug categories, but if one is interested, a good place to start would be the various semgrep rule packs available - or check out gosec. Roughly, most common Go security bugs can be categorised as follows:

  • Cryptographic misconfigurations
  • Input validation (SQLi, RCE, etc.)
  • File/URL path handling
  • File permissions (less common as default permissions are pretty secure since ~v1.17)
  • Concurrency issues (goroutine leaks, race conditions, etc.)
  • Denial of Service
  • Time of check, time of use issues

Rather than going through each of these categories, we will highlight a couple of patterns that either tend to be high impact, obscure, too common, relatively unknown, just interesting or any combination of these.

Also, as a general note for the discussions below: not all bugs might have a security risk, or at least not immediately apparent. To write about these patterns in a generic way, we must abstract a lot of the context away, meaning should the reader run into any of these in a code review, they would have to figure out the impact individually. Sometimes a bug will be just a bug, but in some cases they can turn out to be serious security flaws.

DIRECTORY TRAVERSAL

To ease into some of the more obscure bug classes, let’s start with something relatively simple, but all too common in Go code. Path/directory traversal is a classic vulnerability where an attacker can interact with the server’s file system by supplying malicious input. This usually takes the form of adding multiple “../” or “..”’s to control a file path.

The functions to note here both belong to the path/filepath package, and namely they are:

  • filepath.Join()
  • filepath.Clean()

Demonstrated by several bug reports, filepath.Join() is a common culprit for directory traversal vulnerabilities. The reason might be that the documentation is a little misleading. It states that:

Join joins any number of path elements into a single path, separating them with an OS specific Separator. Empty elements are ignored. The result is Cleaned.

The key word here being “Cleaned”, which is none other than the above mentioned filepath.Clean() function. From the documentation (and source code comments), here is what filepath.Clean() does:

  1. Replace multiple Separator elements with a single one.
  2. Eliminate each . path name element (the current directory).
  3. Eliminate each inner .. path name element (the parent directory) along with the non-.. element that precedes it.
  4. Eliminate .. elements that begin a rooted path: that is, replace “/..” by “/” at the beginning of a path, assuming Separator is ‘/’.

On a cursory glance this could be easily read in a way that ensures that the function protects against directory traversal attacks.

However, if we prepare a quick test program, we can see that it doesn’t exactly do that:

package main

import (
"fmt"
"path/filepath"
)

func main() {
strings := []string{
"/elttam/./../elttam",
"elttam/../elttam",
"..elttam/./../elttam",
"elttam/../../../../../../../elttam/elttam",
}

domain := "elttam.com"

for _, s := range strings {
fmt.Println(filepath.Join(domain, s))
}
}

Output: