Skip to content

Commit

Permalink
*: enable gosec (#35873)
Browse files Browse the repository at this point in the history
ref #35345
  • Loading branch information
hawkingrei authored Jul 5, 2022
1 parent d617ba2 commit 114c922
Show file tree
Hide file tree
Showing 27 changed files with 994 additions and 59 deletions.
819 changes: 783 additions & 36 deletions DEPS.bzl

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions br/pkg/lightning/config/bytesize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ func TestByteSizeTOMLDecode(t *testing.T) {
},
{
input: "x = ['100000']",
err: "toml: cannot load TOML value.*",
err: "toml: incompatible types:.*",
},
{
input: "x = { size = '100000' }",
err: "toml: cannot load TOML value.*",
err: "toml: incompatible types:.*",
},
}

Expand Down
4 changes: 2 additions & 2 deletions br/pkg/lightning/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ func TestInvalidTOML(t *testing.T) {
delimiter = '\'
backslash-escape = true
`))
require.EqualError(t, err, "Near line 2 (last key parsed ''): expected '.' or '=', but got '[' instead")
require.EqualError(t, err, "toml: line 2: expected '.' or '=', but got '[' instead")
}

func TestTOMLUnusedKeys(t *testing.T) {
Expand Down Expand Up @@ -674,7 +674,7 @@ func TestLoadFromInvalidConfig(t *testing.T) {
ConfigFileContent: []byte("invalid toml"),
})
require.Error(t, err)
require.Regexp(t, "Near line 1.*", err.Error())
require.Regexp(t, "line 1.*", err.Error())
}

func TestTomlPostRestore(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions br/pkg/storage/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ func (l *LocalStorage) URI() string {

// Open a Reader by file path, path is a relative path to base path.
func (l *LocalStorage) Open(ctx context.Context, path string) (ExternalFileReader, error) {
//nolint: gosec
return os.Open(filepath.Join(l.base, path))
}

Expand Down
1 change: 1 addition & 0 deletions build/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ nogo(
"//build/linter/exportloopref:exportloopref",
"//build/linter/gofmt:gofmt",
"//build/linter/gci:gci",
"//build/linter/gosec:gosec",
"//build/linter/ineffassign:ineffassign",
"//build/linter/misspell:misspell",
"//build/linter/prealloc:prealloc",
Expand Down
16 changes: 16 additions & 0 deletions build/linter/gosec/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "gosec",
srcs = ["analysis.go"],
importpath = "github.com/pingcap/tidb/build/linter/gosec",
visibility = ["//visibility:public"],
deps = [
"//build/linter/util",
"@com_github_golangci_golangci_lint//pkg/result",
"@com_github_golangci_gosec//:gosec",
"@com_github_golangci_gosec//rules",
"@org_golang_x_tools//go/analysis",
"@org_golang_x_tools//go/loader",
],
)
110 changes: 110 additions & 0 deletions build/linter/gosec/analysis.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// Copyright 2022 PingCAP, Inc.
//
// 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 gosec

import (
"fmt"
"go/token"
"go/types"
"io/ioutil"
"log"
"strconv"

"github.com/golangci/golangci-lint/pkg/result"
"github.com/golangci/gosec"
"github.com/golangci/gosec/rules"
"github.com/pingcap/tidb/build/linter/util"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/loader"
)

// Name is the name of the analyzer.
const Name = "gosec"

// Analyzer is the analyzer struct of gosec.
var Analyzer = &analysis.Analyzer{
Name: Name,
Doc: "Inspects source code for security problems",
Run: run,
}

func init() {
util.SkipAnalyzer(Analyzer)
}

func run(pass *analysis.Pass) (interface{}, error) {
gasConfig := gosec.NewConfig()
enabledRules := rules.Generate(func(id string) bool {
if id == "G104" || id == "G103" || id == "G101" || id == "G201" {
return true
}
return false
})
logger := log.New(ioutil.Discard, "", 0)
analyzer := gosec.NewAnalyzer(gasConfig, logger)
analyzer.LoadRules(enabledRules.Builders())

var createdPkgs []*loader.PackageInfo
createdPkgs = append(createdPkgs, util.MakeFakeLoaderPackageInfo(pass))
allPkgs := make(map[*types.Package]*loader.PackageInfo)
for _, pkg := range createdPkgs {
pkg := pkg
allPkgs[pkg.Pkg] = pkg
}
prog := &loader.Program{
Fset: pass.Fset,
Imported: nil, // not used without .Created in any linter
Created: createdPkgs, // all initial packages
AllPackages: allPkgs, // all initial packages and their depndencies
}

analyzer.ProcessProgram(prog)
issues, _ := analyzer.Report()
if len(issues) == 0 {
return nil, nil
}
severity, confidence := gosec.Low, gosec.Low
issues = filterIssues(issues, severity, confidence)
for _, i := range issues {
fileContent, tf, err := util.ReadFile(pass.Fset, i.File)
if err != nil {
panic(err)
}
text := fmt.Sprintf("[%s] %s: %s", Name, i.RuleID, i.What) // TODO: use severity and confidence
var r *result.Range
line, err := strconv.Atoi(i.Line)
if err != nil {
r = &result.Range{}
if n, rerr := fmt.Sscanf(i.Line, "%d-%d", &r.From, &r.To); rerr != nil || n != 2 {
continue
}
line = r.From
}

pass.Reportf(token.Pos(tf.Base()+util.FindOffset(string(fileContent), line, 1)), text)
}

return nil, nil
}

func filterIssues(issues []*gosec.Issue, severity, confidence gosec.Score) []*gosec.Issue {
res := make([]*gosec.Issue, 0)
for _, issue := range issues {
if issue.Severity >= severity && issue.Confidence >= confidence {
res = append(res, issue)
}
}
return res
}
2 changes: 1 addition & 1 deletion build/linter/misspell/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//build/linter/util",
"@com_github_golangci_misspell//:go_default_library",
"@com_github_golangci_misspell//:misspell",
"@org_golang_x_tools//go/analysis",
],
)
1 change: 1 addition & 0 deletions build/linter/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ func MakeFakeLoaderPackageInfo(pass *analysis.Pass) *loader.PackageInfo {
// ReadFile reads a file and adds it to the FileSet
// so that we can report errors against it using lineStart.
func ReadFile(fset *token.FileSet, filename string) ([]byte, *token.File, error) {
//nolint: gosec
content, err := ioutil.ReadFile(filename)
if err != nil {
return nil, nil, err
Expand Down
16 changes: 16 additions & 0 deletions build/nogo_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,22 @@
"util/printer/printer.go": "ignore util/printer code"
}
},
"gosec": {
"exclude_files": {
"external/": "no need to vet third party code",
"parser/goyacc/": "ignore goyacc code",
".*_test\\.go$": "ignore generated code",
"/cgo/": "ignore cgo code",
"/rules_go_work-*": "ignore generated code",
"tools/check/ut.go": "ignore tools/check code",
"tools/check/xprog.go": "ignore tools/check code",
"cmd/pluginpkg/pluginpkg.go": "ignore cmd/pluginpkg code",
"tools/check/xprog.go:": "ignore tools/check code",
"cmd/explaintest/main.go": "ignore cmd/explaintest code",
"GOROOT/": "ignore code",
".*_generated\\.go$": "ignore generated code"
}
},
"httpresponse": {
"exclude_files": {
"/external/": "no need to vet third party code",
Expand Down
2 changes: 2 additions & 0 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -5962,10 +5962,12 @@ func buildFKInfo(fkName model.CIStr, keys []*ast.IndexPartSpecification, refer *
// Check wrong reference options of foreign key on stored generated columns
switch refer.OnUpdate.ReferOpt {
case ast.ReferOptionCascade, ast.ReferOptionSetNull, ast.ReferOptionSetDefault:
//nolint: gosec
return nil, dbterror.ErrWrongFKOptionForGeneratedColumn.GenWithStackByArgs("ON UPDATE " + refer.OnUpdate.ReferOpt.String())
}
switch refer.OnDelete.ReferOpt {
case ast.ReferOptionSetNull, ast.ReferOptionSetDefault:
//nolint: gosec
return nil, dbterror.ErrWrongFKOptionForGeneratedColumn.GenWithStackByArgs("ON DELETE " + refer.OnDelete.ReferOpt.String())
}
continue
Expand Down
1 change: 1 addition & 0 deletions executor/plan_replayer.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ func (e *PlanReplayerSingleExec) dumpSingle(path string) (fileName string, err e
// Generate key and create zip file
time := time.Now().UnixNano()
b := make([]byte, 16)
//nolint: gosec
_, err = rand.Read(b)
if err != nil {
return "", err
Expand Down
1 change: 1 addition & 0 deletions executor/slow_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,7 @@ func (e *slowQueryRetriever) getAllFiles(ctx context.Context, sctx sessionctx.Co
}
if e.extractor == nil || !e.extractor.Enable {
totalFileNum = 1
//nolint: gosec
file, err := os.Open(logFilePath)
if err != nil {
if os.IsNotExist(err) {
Expand Down
1 change: 1 addition & 0 deletions executor/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ func generateOptimizerTraceFile() (*os.File, string, error) {
// Generate key and create zip file
time := time.Now().UnixNano()
b := make([]byte, 16)
//nolint: gosec
_, err = rand.Read(b)
if err != nil {
return nil, "", errors.AddStack(err)
Expand Down
1 change: 1 addition & 0 deletions expression/builtin_encryption.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,7 @@ func (b *builtinRandomBytesSig) evalString(row chunk.Row) (string, bool, error)
return "", false, types.ErrOverflow.GenWithStackByArgs("length", "random_bytes")
}
buf := make([]byte, val)
//nolint: gosec
if n, err := rand.Read(buf); err != nil {
return "", true, err
} else if int64(n) != val {
Expand Down
18 changes: 10 additions & 8 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
cloud.google.com/go/storage v1.21.0
github.com/Azure/azure-sdk-for-go/sdk/azidentity v0.12.0
github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v0.2.0
github.com/BurntSushi/toml v0.4.1
github.com/BurntSushi/toml v1.1.0
github.com/DATA-DOG/go-sqlmock v1.5.0
github.com/Jeffail/gabs/v2 v2.5.1
github.com/Shopify/sarama v1.29.0
Expand Down Expand Up @@ -54,7 +54,7 @@ require (
github.com/prometheus/client_golang v1.12.2
github.com/prometheus/client_model v0.2.0
github.com/prometheus/common v0.32.1
github.com/shirou/gopsutil/v3 v3.21.12
github.com/shirou/gopsutil/v3 v3.22.4
github.com/shurcooL/httpgzip v0.0.0-20190720172056-320755c1c1b0
github.com/shurcooL/vfsgen v0.0.0-20200824052919-0d455de96546 // indirect
github.com/soheilhy/cmux v0.1.5
Expand Down Expand Up @@ -100,19 +100,22 @@ require (
github.com/charithe/durationcheck v0.0.9
github.com/daixiang0/gci v0.3.4
github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a
github.com/golangci/golangci-lint v1.46.2
github.com/golangci/gosec v0.0.0-20180901114220-8afd9cbb6cfb
github.com/golangci/misspell v0.3.5
github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4
github.com/gordonklaus/ineffassign v0.0.0-20210914165742-4cc7213b9bc8
github.com/kisielk/errcheck v1.6.1
github.com/kyoh86/exportloopref v0.1.8
github.com/nishanths/predeclared v0.2.2
github.com/tdakkota/asciicheck v0.1.1
honnef.co/go/tools v0.0.1-2020.1.4
honnef.co/go/tools v0.3.1
)

require (
github.com/hexops/gotextdiff v1.0.3 // indirect
github.com/kisielk/gotool v1.0.0 // indirect
github.com/nbutton23/zxcvbn-go v0.0.0-20210217022336-fa2cb2858354 // indirect
)

require (
Expand Down Expand Up @@ -142,7 +145,6 @@ require (
github.com/eapache/queue v1.1.0 // indirect
github.com/felixge/httpsnoop v1.0.1 // indirect
github.com/form3tech-oss/jwt-go v3.2.5+incompatible // indirect
github.com/fsnotify/fsnotify v1.5.1 // indirect
github.com/go-ole/go-ole v1.2.6 // indirect
github.com/golang/glog v1.0.0 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
Expand Down Expand Up @@ -190,8 +192,8 @@ require (
github.com/shurcooL/httpfs v0.0.0-20190707220628-8d4bc4ba7749 // indirect
github.com/sirupsen/logrus v1.8.1 // indirect
github.com/stathat/consistent v1.0.0 // indirect
github.com/tklauser/go-sysconf v0.3.9 // indirect
github.com/tklauser/numcpus v0.3.0 // indirect
github.com/tklauser/go-sysconf v0.3.10 // indirect
github.com/tklauser/numcpus v0.4.0 // indirect
github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802 // indirect
github.com/uber/jaeger-lib v2.4.1+incompatible // indirect
github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 // indirect
Expand All @@ -214,10 +216,10 @@ require (
golang.org/x/crypto v0.0.0-20220214200702-86341886e292 // indirect
golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e // indirect
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
golang.org/x/xerrors v0.0.0-20220411194840-2f41105eb62f // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20220216160803-4663080d8bc8 // indirect
google.golang.org/protobuf v1.27.1 // indirect
google.golang.org/protobuf v1.28.0 // indirect
gopkg.in/natefinch/lumberjack.v2 v2.0.0 // indirect
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
sigs.k8s.io/yaml v1.2.0 // indirect
Expand Down
Loading

0 comments on commit 114c922

Please sign in to comment.