Add G110(Potential DoS vulnerability via decompression bomb)

Signed-off-by: Hiroki Suezawa <suezawa@gmail.com>

README.md

@@ -72,6 +72,7 @@ directory you can supply `./...` as the input argument.
 - G107: Url provided to HTTP request as taint input
 - G108: Profiling endpoint automatically exposed on /debug/pprof
 - G109: Potential Integer overflow made by strconv.Atoi result conversion to int16/32
+- G110: Potential DoS vulnerability via decompression bomb
 - G201: SQL query construction using format string
 - G202: SQL query construction using string concatenation
 - G203: Use of unescaped data in HTML templates

issue.go

@@ -54,6 +54,7 @@ var IssueToCWE = map[string]Cwe{
 	"G106": GetCwe("322"),
 	"G107": GetCwe("88"),
 	"G109": GetCwe("190"),
+	"G110": GetCwe("409"),
 	"G201": GetCwe("89"),
 	"G202": GetCwe("89"),
 	"G203": GetCwe("79"),

output/formatter_test.go

@@ -251,9 +251,9 @@ var _ = Describe("Formatter", func() {
 	Context("When using different report formats", func() {
 
 		grules := []string{"G101", "G102", "G103", "G104", "G106",
-			"G107", "G109", "G201", "G202", "G203", "G204",
+			"G107", "G109", "G110", "G201", "G202", "G203", "G204",
-			"G301", "G302", "G303", "G304", "G305", "G401",
+			"G301", "G302", "G303", "G304", "G305", "G401", "G402",
-			"G402", "G403", "G404", "G501", "G502", "G503", "G504", "G505"}
+			"G403", "G404", "G501", "G502", "G503", "G504", "G505"}
 
 		It("csv formatted report should contain the CWE mapping", func() {
 			for _, rule := range grules {

rules/decompression-bomb.go

@@ -0,0 +1,111 @@
+// (c) Copyright 2016 Hewlett Packard Enterprise Development LP
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package rules
+
+import (
+	"fmt"
+	"github.com/securego/gosec"
+	"go/ast"
+)
+
+type decompressionBombCheck struct {
+	gosec.MetaData
+	readerCalls gosec.CallList
+	copyCalls   gosec.CallList
+}
+
+func (d *decompressionBombCheck) ID() string {
+	return d.MetaData.ID
+}
+
+func containsReaderCall(node ast.Node, ctx *gosec.Context, list gosec.CallList) bool {
+	if list.ContainsCallExpr(node, ctx, false) != nil {
+		return true
+	}
+	// Resolve type info of ident (for *archive/zip.File.Open)
+	s, idt, _ := gosec.GetCallInfo(node, ctx)
+	if list.Contains(s, idt) {
+		return true
+	}
+	return false
+}
+
+func (d *decompressionBombCheck) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, error) {
+	var readerVarObj map[*ast.Object]struct{}
+
+	// To check multiple lines, ctx.PassedValues is used to store temporary data.
+	if _, ok := ctx.PassedValues[d.ID()]; !ok {
+		readerVarObj = make(map[*ast.Object]struct{})
+		ctx.PassedValues[d.ID()] = readerVarObj
+	} else if pv, ok := ctx.PassedValues[d.ID()].(map[*ast.Object]struct{}); ok {
+		readerVarObj = pv
+	} else {
+		return nil, fmt.Errorf("PassedValues[%s] of Context is not map[*ast.Object]struct{}, but %T", d.ID(), ctx.PassedValues[d.ID()])
+	}
+
+	// io.Copy is a common function.
+	// To reduce false positives, This rule detects code which is used for compressed data only.
+	switch n := node.(type) {
+	case *ast.AssignStmt:
+		for _, expr := range n.Rhs {
+			if callExpr, ok := expr.(*ast.CallExpr); ok && containsReaderCall(callExpr, ctx, d.readerCalls) {
+				if idt, ok := n.Lhs[0].(*ast.Ident); ok && idt.Name != "_" {
+					// Example:
+					//  r, _ := zlib.NewReader(buf)
+					//  Add r's Obj to readerVarObj map
+					readerVarObj[idt.Obj] = struct{}{}
+				}
+			}
+		}
+	case *ast.CallExpr:
+		if d.copyCalls.ContainsCallExpr(n, ctx, false) != nil {
+			if idt, ok := n.Args[1].(*ast.Ident); ok {
+				if _, ok := readerVarObj[idt.Obj]; ok {
+					// Detect io.Copy(x, r)
+					return gosec.NewIssue(ctx, n, d.ID(), d.What, d.Severity, d.Confidence), nil
+				}
+			}
+		}
+	}
+
+	return nil, nil
+}
+
+// NewDecompressionBombCheck detects if there is potential DoS vulnerability via decompression bomb
+func NewDecompressionBombCheck(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
+	readerCalls := gosec.NewCallList()
+	readerCalls.Add("compress/gzip", "NewReader")
+	readerCalls.AddAll("compress/zlib", "NewReader", "NewReaderDict")
+	readerCalls.Add("compress/bzip2", "NewReader")
+	readerCalls.AddAll("compress/flate", "NewReader", "NewReaderDict")
+	readerCalls.Add("compress/lzw", "NewReader")
+	readerCalls.Add("archive/tar", "NewReader")
+	readerCalls.Add("archive/zip", "NewReader")
+	readerCalls.Add("*archive/zip.File", "Open")
+
+	copyCalls := gosec.NewCallList()
+	copyCalls.Add("io", "Copy")
+
+	return &decompressionBombCheck{
+		MetaData: gosec.MetaData{
+			ID:         id,
+			Severity:   gosec.Medium,
+			Confidence: gosec.Medium,
+			What:       "Potential DoS vulnerability via decompression bomb",
+		},
+		readerCalls: readerCalls,
+		copyCalls:   copyCalls,
+	}, []ast.Node{(*ast.FuncDecl)(nil), (*ast.AssignStmt)(nil), (*ast.CallExpr)(nil)}
+}

rules/rulelist.go

@@ -67,6 +67,7 @@ func Generate(filters ...RuleFilter) RuleList {
 		{"G107", "Url provided to HTTP request as taint input", NewSSRFCheck},
 		{"G108", "Profiling endpoint is automatically exposed", NewPprofCheck},
 		{"G109", "Converting strconv.Atoi result to int32/int16", NewIntegerOverflowCheck},
+		{"G110", "Detect io.Copy instead of io.CopyN when decompression", NewDecompressionBombCheck},
 
 		// injection
 		{"G201", "SQL query construction using format string", NewSQLStrFormat},

rules/rules_test.go

@@ -87,6 +87,10 @@ var _ = Describe("gosec rules", func() {
 			runner("G109", testutils.SampleCodeG109)
 		})
 
+		It("should detect DoS vulnerability via decompression bomb", func() {
+			runner("G110", testutils.SampleCodeG110)
+		})
+
 		It("should detect sql injection via format strings", func() {
 			runner("G201", testutils.SampleCodeG201)
 		})

testutils/source.go

@@ -594,6 +594,94 @@ func test() {
 	fmt.Println(value)
 }`}, 0, gosec.NewConfig()}}
 
+	// SampleCodeG110 - potential DoS vulnerability via decompression bomb
+	SampleCodeG110 = []CodeSample{
+		{[]string{`
+package main
+
+import (
+	"bytes"
+	"compress/zlib"
+	"io"
+	"os"
+)
+
+func main() {
+	buff := []byte{120, 156, 202, 72, 205, 201, 201, 215, 81, 40, 207,
+		47, 202, 73, 225, 2, 4, 0, 0, 255, 255, 33, 231, 4, 147}
+	b := bytes.NewReader(buff)
+
+	r, err := zlib.NewReader(b)
+	if err != nil {
+		panic(err)
+	}
+	io.Copy(os.Stdout, r)
+
+	r.Close()
+}`}, 1, gosec.NewConfig()}, {[]string{`
+package main
+
+import (
+	"archive/zip"
+	"io"
+	"os"
+	"strconv"
+)
+
+func main() {
+	r, err := zip.OpenReader("tmp.zip")
+	if err != nil {
+		panic(err)
+	}
+	defer r.Close()
+
+	for i, f := range r.File {
+		out, err := os.OpenFile("output" + strconv.Itoa(i), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
+		if err != nil {
+			panic(err)
+		}
+
+		rc, err := f.Open()
+		if err != nil {
+			panic(err)
+		}
+
+		_, err = io.Copy(out, rc)
+
+		out.Close()
+		rc.Close()
+
+		if err != nil {
+			panic(err)
+		}
+	}
+}`}, 1, gosec.NewConfig()}, {[]string{`
+package main
+
+import (
+	"io"
+	"os"
+)
+
+func main() {
+	s, err := os.Open("src")
+	if err != nil {
+		panic(err)
+	}
+	defer s.Close()
+
+	d, err := os.Create("dst")
+	if err != nil {
+		panic(err)
+	}
+	defer d.Close()
+
+	_, err = io.Copy(d, s)
+	if  err != nil {
+		panic(err)
+	}
+}`}, 0, gosec.NewConfig()}}
+
 	// SampleCodeG201 - SQL injection via format string
 	SampleCodeG201 = []CodeSample{
 		{[]string{`