mirror of https://github.com/golang/go.git
internal/lsp/command: let RunVulncheckExp call gopls vulncheck
By making gopls.run_vulncheck_exp (RunVulncheckExp implements) call `gopls vulncheck`, we achieve - gopls.run_vulncheck_exp can run asynchronously and be cancellable - log information can be forwarded as progress messages - isolate any failures during vulncheck execution In this CL, we also changed not to include test files in the analysis (match the default of govulncheck). We will add an option in the future. TODO: - prevent concurrent gopls.run_vulncheck_exp - convert the gopls vulncheck output to diagnostics and publish it - remove timestamps from the `gopls vulncheck` log messages for simplify progress messages - add test to check vulnerability in third-party dependencies Change-Id: I21592e03794cd9e9d96ed3989973a2ab7d75c538 Reviewed-on: https://go-review.googlesource.com/c/tools/+/420717 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Suzy Mueller <suzmue@golang.org> Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com> gopls-CI: kokoro <noreply+kokoro@google.com>
This commit is contained in:
Hana (Hyang-Ah) Kim
Hyang-Ah Hana Kim
parent
3e0a5031e3
commit
bceee4b059
|
|
@ -281,26 +281,6 @@ Args:
|
|||
}
|
||||
```
|
||||
|
||||
Result:
|
||||
|
||||
```
|
||||
{
|
||||
"Vuln": []{
|
||||
"ID": string,
|
||||
"Details": string,
|
||||
"Aliases": []string,
|
||||
"Symbol": string,
|
||||
"PkgPath": string,
|
||||
"ModPath": string,
|
||||
"URL": string,
|
||||
"CurrentVersion": string,
|
||||
"FixedVersion": string,
|
||||
"CallStacks": [][]golang.org/x/tools/internal/lsp/command.StackEntry,
|
||||
"CallStackSummaries": []string,
|
||||
},
|
||||
}
|
||||
```
|
||||
|
||||
### **Start the gopls debug server**
|
||||
Identifier: `gopls.start_debugging`
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1 @@
|
|||
[{"id":"GO-2020-0012","published":"2021-04-14T20:04:52Z","modified":"2021-04-14T20:04:52Z","aliases":["CVE-2020-9283"],"details":"An attacker can craft an ssh-ed25519 or sk-ssh-ed25519@openssh.com public\nkey, such that the library will panic when trying to verify a signature\nwith it. If verifying signatures using user supplied public keys, this\nmay be used as a denial of service vector.\n","affected":[{"package":{"name":"golang.org/x/crypto/ssh","ecosystem":"Go"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"0.0.0-20200220183623-bac4c82f6975"}]}],"database_specific":{"url":"https://pkg.go.dev/vuln/GO-2020-0012"},"ecosystem_specific":{"symbols":["parseED25519","ed25519PublicKey.Verify","parseSKEd25519","skEd25519PublicKey.Verify","NewPublicKey"]}}],"references":[{"type":"FIX","url":"https://go-review.googlesource.com/c/crypto/+/220357"},{"type":"FIX","url":"https://go.googlesource.com/crypto/+/bac4c82f69751a6dd76e702d54b3ceb88adab236"},{"type":"WEB","url":"https://groups.google.com/g/golang-announce/c/3L45YRc91SY"}]},{"id":"GO-2020-0013","published":"2021-04-14T20:04:52Z","modified":"2021-04-14T20:04:52Z","aliases":["CVE-2017-3204"],"details":"By default host key verification is disabled which allows for\nman-in-the-middle attacks against SSH clients if\nClientConfig.HostKeyCallback is not set.\n","affected":[{"package":{"name":"golang.org/x/crypto/ssh","ecosystem":"Go"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"0.0.0-20170330155735-e4e2799dd7aa"}]}],"database_specific":{"url":"https://pkg.go.dev/vuln/GO-2020-0013"},"ecosystem_specific":{"symbols":["NewClientConn"]}}],"references":[{"type":"FIX","url":"https://go-review.googlesource.com/38701"},{"type":"FIX","url":"https://go.googlesource.com/crypto/+/e4e2799dd7aab89f583e1d898300d96367750991"},{"type":"WEB","url":"https://go.dev/issue/19767"},{"type":"WEB","url":"https://bridge.grumpy-troll.org/2017/04/golang-ssh-security/"}]},{"id":"GO-2021-0227","published":"2022-02-17T17:35:32Z","modified":"2022-02-17T17:35:32Z","aliases":["CVE-2020-29652"],"details":"Clients can cause a panic in SSH servers. An attacker can craft\nan authentication request message for the “gssapi-with-mic” method\nwhich will cause NewServerConn to panic via a nil pointer dereference\nif ServerConfig.GSSAPIWithMICConfig is nil.\n","affected":[{"package":{"name":"golang.org/x/crypto/ssh","ecosystem":"Go"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"0.0.0-20201216223049-8b5274cf687f"}]}],"database_specific":{"url":"https://pkg.go.dev/vuln/GO-2021-0227"},"ecosystem_specific":{"symbols":["connection.serverAuthenticate"]}}],"references":[{"type":"FIX","url":"https://go-review.googlesource.com/c/crypto/+/278852"},{"type":"FIX","url":"https://go.googlesource.com/crypto/+/8b5274cf687fd9316b4108863654cc57385531e8"},{"type":"WEB","url":"https://groups.google.com/g/golang-announce/c/ouZIlBimOsE?pli=1"}]}]
|
||||
|
|
@ -0,0 +1 @@
|
|||
[{"id":"GO-2020-0015","published":"2021-04-14T20:04:52Z","modified":"2021-06-07T12:00:00Z","aliases":["CVE-2020-14040"],"details":"An attacker could provide a single byte to a UTF16 decoder instantiated with\nUseBOM or ExpectBOM to trigger an infinite loop if the String function on\nthe Decoder is called, or the Decoder is passed to transform.String.\nIf used to parse user supplied input, this may be used as a denial of service\nvector.\n","affected":[{"package":{"name":"golang.org/x/text/encoding/unicode","ecosystem":"Go"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"0.3.3"}]}],"database_specific":{"url":"https://pkg.go.dev/vuln/GO-2020-0015"},"ecosystem_specific":{"symbols":["utf16Decoder.Transform","bomOverride.Transform"]}},{"package":{"name":"golang.org/x/text/transform","ecosystem":"Go"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"0.3.3"}]}],"database_specific":{"url":"https://pkg.go.dev/vuln/GO-2020-0015"},"ecosystem_specific":{"symbols":["Transform"]}}],"references":[{"type":"FIX","url":"https://go-review.googlesource.com/c/text/+/238238"},{"type":"FIX","url":"https://go.googlesource.com/text/+/23ae387dee1f90d29a23c0e87ee0b46038fbed0e"},{"type":"WEB","url":"https://go.dev/issue/39491"},{"type":"WEB","url":"https://groups.google.com/g/golang-announce/c/bXVeAmGOqz0"}]},{"id":"GO-2021-0113","published":"2021-10-06T17:51:21Z","modified":"2021-10-06T17:51:21Z","aliases":["CVE-2021-38561"],"details":"Due to improper index calculation, an incorrectly formatted language tag can cause Parse\nto panic via an out of bounds read. If Parse is used to process untrusted user inputs,\nthis may be used as a vector for a denial of service attack.\n","affected":[{"package":{"name":"golang.org/x/text/language","ecosystem":"Go"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"0.3.7"}]}],"database_specific":{"url":"https://pkg.go.dev/vuln/GO-2021-0113"},"ecosystem_specific":{"symbols":["Parse","MatchStrings","MustParse","ParseAcceptLanguage"]}}],"references":[{"type":"FIX","url":"https://go-review.googlesource.com/c/text/+/340830"},{"type":"FIX","url":"https://go.googlesource.com/text/+/383b2e75a7a4198c42f8f87833eefb772868a56f"}]}]
|
||||
|
|
@ -0,0 +1 @@
|
|||
[{"id":"STD","affected":[{"package":{"name":"archive/zip"},"ranges":[{"type":"SEMVER","events":[{"introduced":"1.18.0"}]}],"ecosystem_specific":{"symbols":["OpenReader"]}}]}]
|
||||
|
|
@ -5,11 +5,14 @@
|
|||
package misc
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"golang.org/x/tools/internal/lsp/command"
|
||||
"golang.org/x/tools/internal/lsp/protocol"
|
||||
. "golang.org/x/tools/internal/lsp/regtest"
|
||||
"golang.org/x/tools/internal/testenv"
|
||||
)
|
||||
|
||||
func TestRunVulncheckExpError(t *testing.T) {
|
||||
|
|
@ -41,3 +44,56 @@ package foo
|
|||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestRunVulncheckExp(t *testing.T) {
|
||||
testenv.NeedsGo1Point(t, 18)
|
||||
const files = `
|
||||
-- go.mod --
|
||||
module mod.com
|
||||
|
||||
go 1.18
|
||||
-- main.go --
|
||||
package main
|
||||
|
||||
import (
|
||||
"archive/zip"
|
||||
"fmt"
|
||||
)
|
||||
|
||||
func main() {
|
||||
_, err := zip.OpenReader("file.zip") // vulnerable.
|
||||
fmt.Println(err)
|
||||
}
|
||||
`
|
||||
|
||||
cwd, _ := os.Getwd()
|
||||
WithOptions(
|
||||
EnvVars{
|
||||
// Let the analyzer read vulnerabilities data from the testdata/vulndb.
|
||||
"GOVULNDB": "file://" + filepath.Join(cwd, "testdata", "vulndb"),
|
||||
// When fetchinging stdlib package vulnerability info,
|
||||
// behave as if our go version is go1.18 for this testing.
|
||||
// The default behavior is to run `go env GOVERSION` (which isn't mutable env var).
|
||||
// See gopls/internal/vulncheck.goVersion
|
||||
// which follows the convention used in golang.org/x/vuln/cmd/govulncheck.
|
||||
"GOVERSION": "go1.18",
|
||||
"_GOPLS_TEST_BINARY_RUN_AS_GOPLS": "true",
|
||||
},
|
||||
).Run(t, files, func(t *testing.T, env *Env) {
|
||||
cmd, err := command.NewRunVulncheckExpCommand("Run Vulncheck Exp", command.VulncheckArgs{
|
||||
Dir: env.Sandbox.Workdir.RootURI(),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
env.ExecuteCommand(&protocol.ExecuteCommandParams{
|
||||
Command: command.RunVulncheckExp.ID(),
|
||||
Arguments: cmd.Arguments,
|
||||
}, nil)
|
||||
env.Await(
|
||||
CompletedWork("Checking vulnerability", 1, true),
|
||||
// TODO(hyangah): once the diagnostics are published, wait for diagnostics.
|
||||
)
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -13,13 +13,13 @@ import (
|
|||
"io"
|
||||
"io/ioutil"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
"golang.org/x/mod/modfile"
|
||||
"golang.org/x/tools/go/ast/astutil"
|
||||
"golang.org/x/tools/go/packages"
|
||||
"golang.org/x/tools/internal/event"
|
||||
"golang.org/x/tools/internal/gocommand"
|
||||
"golang.org/x/tools/internal/lsp/command"
|
||||
|
|
@ -790,9 +790,29 @@ func (c *commandHandler) StartDebugging(ctx context.Context, args command.Debugg
|
|||
return result, nil
|
||||
}
|
||||
|
||||
func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.VulncheckArgs) (result command.VulncheckResult, _ error) {
|
||||
// Copy of pkgLoadConfig defined in internal/lsp/cmd/vulncheck.go
|
||||
// TODO(hyangah): decide where to define this.
|
||||
type pkgLoadConfig struct {
|
||||
// BuildFlags is a list of command-line flags to be passed through to
|
||||
// the build system's query tool.
|
||||
BuildFlags []string
|
||||
|
||||
// Env is the environment to use when invoking the build system's query tool.
|
||||
// If Env is nil, the current environment is used.
|
||||
// TODO: This seems unnecessary. Delete.
|
||||
Env []string
|
||||
|
||||
// If Tests is set, the loader includes related test packages.
|
||||
Tests bool
|
||||
}
|
||||
|
||||
func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.VulncheckArgs) error {
|
||||
if args.Dir == "" {
|
||||
return errors.New("VulncheckArgs is missing Dir field")
|
||||
}
|
||||
err := c.run(ctx, commandConfig{
|
||||
progress: "Running vulncheck",
|
||||
async: true, // need to be async to be cancellable
|
||||
progress: "Checking vulnerability",
|
||||
requireSave: true,
|
||||
forURI: args.Dir, // Will dir work?
|
||||
}, func(ctx context.Context, deps commandDeps) error {
|
||||
|
|
@ -802,22 +822,44 @@ func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.Vulnc
|
|||
return errors.New("vulncheck feature is not available")
|
||||
}
|
||||
|
||||
buildFlags := opts.BuildFlags // XXX: is session.Options equivalent to view.Options?
|
||||
cmd := exec.Command(os.Args[0], "vulncheck", "-config", args.Pattern)
|
||||
cmd.Dir = args.Dir.SpanURI().Filename()
|
||||
|
||||
var viewEnv []string
|
||||
if e := opts.EnvSlice(); e != nil {
|
||||
viewEnv = append(os.Environ(), e...)
|
||||
}
|
||||
cfg := &packages.Config{
|
||||
Context: ctx,
|
||||
Tests: true, // TODO(hyangah): add a field in args.
|
||||
BuildFlags: buildFlags,
|
||||
Env: viewEnv,
|
||||
Dir: args.Dir.SpanURI().Filename(),
|
||||
// TODO(hyangah): configure overlay
|
||||
cmd.Env = viewEnv
|
||||
|
||||
// stdin: gopls vulncheck expects JSON-encoded configuration from STDIN when -config flag is set.
|
||||
var stdin bytes.Buffer
|
||||
cmd.Stdin = &stdin
|
||||
|
||||
if err := json.NewEncoder(&stdin).Encode(pkgLoadConfig{
|
||||
BuildFlags: opts.BuildFlags,
|
||||
// TODO(hyangah): add `tests` flag in command.VulncheckArgs
|
||||
}); err != nil {
|
||||
return fmt.Errorf("failed to pass package load config: %v", err)
|
||||
}
|
||||
var err error
|
||||
result, err = opts.Hooks.Govulncheck(ctx, cfg, args)
|
||||
return err
|
||||
|
||||
// stderr: stream gopls vulncheck's STDERR as progress reports
|
||||
er := progress.NewEventWriter(ctx, "vulncheck")
|
||||
stderr := io.MultiWriter(er, progress.NewWorkDoneWriter(ctx, deps.work))
|
||||
cmd.Stderr = stderr
|
||||
// TODO: can we stream stdout?
|
||||
stdout, err := cmd.Output()
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to run govulncheck: %v", err)
|
||||
}
|
||||
|
||||
var vulns command.VulncheckResult
|
||||
if err := json.Unmarshal(stdout, &vulns); err != nil {
|
||||
// TODO: for easy debugging, log the failed stdout somewhere?
|
||||
return fmt.Errorf("failed to parse govulncheck output: %v", err)
|
||||
}
|
||||
|
||||
// TODO(hyangah): convert the results to diagnostics & code actions.
|
||||
return nil
|
||||
})
|
||||
return result, err
|
||||
return err
|
||||
}
|
||||
|
|
|
|||
|
|
@ -159,7 +159,7 @@ func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Inte
|
|||
if err := UnmarshalArgs(params.Arguments, &a0); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return s.RunVulncheckExp(ctx, a0)
|
||||
return nil, s.RunVulncheckExp(ctx, a0)
|
||||
case "gopls.start_debugging":
|
||||
var a0 DebuggingArgs
|
||||
if err := UnmarshalArgs(params.Arguments, &a0); err != nil {
|
||||
|
|
|
|||
|
|
@ -147,7 +147,7 @@ type Interface interface {
|
|||
// RunVulncheckExp: Run vulncheck (experimental)
|
||||
//
|
||||
// Run vulnerability check (`govulncheck`).
|
||||
RunVulncheckExp(context.Context, VulncheckArgs) (VulncheckResult, error)
|
||||
RunVulncheckExp(context.Context, VulncheckArgs) error
|
||||
}
|
||||
|
||||
type RunTestsArgs struct {
|
||||
|
|
@ -320,8 +320,7 @@ type VulncheckArgs struct {
|
|||
// Package pattern. E.g. "", ".", "./...".
|
||||
Pattern string
|
||||
|
||||
// TODO: Flag []string (flags accepted by govulncheck, e.g., -tests)
|
||||
// TODO: Format string (json, text)
|
||||
// TODO: -tests
|
||||
}
|
||||
|
||||
type VulncheckResult struct {
|
||||
|
|
|
|||
|
|
@ -726,11 +726,10 @@ var GeneratedAPIJSON = &APIJSON{
|
|||
ArgDoc: "{\n\t// The test file containing the tests to run.\n\t\"URI\": string,\n\t// Specific test names to run, e.g. TestFoo.\n\t\"Tests\": []string,\n\t// Specific benchmarks to run, e.g. BenchmarkFoo.\n\t\"Benchmarks\": []string,\n}",
|
||||
},
|
||||
{
|
||||
Command: "gopls.run_vulncheck_exp",
|
||||
Title: "Run vulncheck (experimental)",
|
||||
Doc: "Run vulnerability check (`govulncheck`).",
|
||||
ArgDoc: "{\n\t// Dir is the directory from which vulncheck will run from.\n\t\"Dir\": string,\n\t// Package pattern. E.g. \"\", \".\", \"./...\".\n\t\"Pattern\": string,\n}",
|
||||
ResultDoc: "{\n\t\"Vuln\": []{\n\t\t\"ID\": string,\n\t\t\"Details\": string,\n\t\t\"Aliases\": []string,\n\t\t\"Symbol\": string,\n\t\t\"PkgPath\": string,\n\t\t\"ModPath\": string,\n\t\t\"URL\": string,\n\t\t\"CurrentVersion\": string,\n\t\t\"FixedVersion\": string,\n\t\t\"CallStacks\": [][]golang.org/x/tools/internal/lsp/command.StackEntry,\n\t\t\"CallStackSummaries\": []string,\n\t},\n}",
|
||||
Command: "gopls.run_vulncheck_exp",
|
||||
Title: "Run vulncheck (experimental)",
|
||||
Doc: "Run vulnerability check (`govulncheck`).",
|
||||
ArgDoc: "{\n\t// Dir is the directory from which vulncheck will run from.\n\t\"Dir\": string,\n\t// Package pattern. E.g. \"\", \".\", \"./...\".\n\t\"Pattern\": string,\n}",
|
||||
},
|
||||
{
|
||||
Command: "gopls.start_debugging",
|
||||
|
|
|
|||
Loading…
Reference in New Issue