Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: enable gosec #35873

Merged
merged 5 commits into from
Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:.*",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some linter needs to upgrade the version of toml. so it change some error message.

},
{
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