diff --git a/README.md b/README.md index 1317ce9..0b02d77 100644 --- a/README.md +++ b/README.md @@ -594,6 +594,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`unnecessary-format`](./RULES_DESCRIPTIONS.md#unnecessary-format) | n/a | Identifies calls to formatting functions where the format string does not contain any formatting verbs | no | no | | [`unnecessary-stmt`](./RULES_DESCRIPTIONS.md#unnecessary-stmt) | n/a | Suggests removing or simplifying unnecessary statements | no | no | | [`unreachable-code`](./RULES_DESCRIPTIONS.md#unreachable-code) | n/a | Warns on unreachable code | no | no | +| [`unsecure-url-scheme`](./RULES_DESCRIPTIONS.md#unsecure-url-scheme) | n/a | Checks for usage of potentially unsecure URL schemes. | no | no | | [`unused-parameter`](./RULES_DESCRIPTIONS.md#unused-parameter) | n/a | Suggests to rename or remove unused function parameters | no | no | | [`unused-receiver`](./RULES_DESCRIPTIONS.md#unused-receiver) | n/a | Suggests to rename or remove unused method receivers | no | no | | [`use-any`](./RULES_DESCRIPTIONS.md#use-any) | n/a | Proposes to replace `interface{}` with its alias `any` | no | no | diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 4e3080d..2f6adcc 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -88,6 +88,7 @@ List of all available rules. - [unnecessary-format](#unnecessary-format) - [unnecessary-stmt](#unnecessary-stmt) - [unreachable-code](#unreachable-code) + - [unsecure-url-scheme](#unsecure-url-scheme) - [unused-parameter](#unused-parameter) - [unused-receiver](#unused-receiver) - [use-any](#use-any) @@ -1351,6 +1352,17 @@ _Description_: This rule spots and proposes to remove [unreachable code](https:/ _Configuration_: N/A +## unsecure-url-scheme + +_Description_: Checks for usage of potentially unsecure URL schemes (`http`, `ws`) in string literals. +Using unencrypted URL schemes can expose sensitive data during transmission and +make applications vulnerable to man-in-the-middle attacks. +Secure alternatives like `https` should be preferred when possible. + +_Configuration_: N/A + +The rule will not warn on local URLs (`localhost`, `127.0.0.1`). + ## unused-parameter _Description_: This rule warns on unused parameters. Functions or methods with unused parameters can be a symptom of an unfinished refactoring or a bug. diff --git a/config/config.go b/config/config.go index 0ffaaab..6a00f03 100644 --- a/config/config.go +++ b/config/config.go @@ -113,6 +113,7 @@ var allRules = append([]lint.Rule{ &rule.UselessFallthroughRule{}, &rule.PackageDirectoryMismatchRule{}, &rule.UseWaitGroupGoRule{}, + &rule.UnsecureURLSchemeRule{}, }, defaultRules...) // allFormatters is a list of all available formatters to output the linting results. diff --git a/rule/unsecure_url_scheme.go b/rule/unsecure_url_scheme.go new file mode 100644 index 0000000..f1ebdba --- /dev/null +++ b/rule/unsecure_url_scheme.go @@ -0,0 +1,88 @@ +package rule + +import ( + "fmt" + "go/ast" + "go/token" + "strconv" + "strings" + + "github.com/mgechev/revive/lint" +) + +// UnsecureURLSchemeRule checks if a file contains string literals with unsecure URL schemes (for example: http://... in place of https://...). +type UnsecureURLSchemeRule struct{} + +// Apply applied the rule to the given file. +func (*UnsecureURLSchemeRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { + if file.IsTest() { + return nil // skip test files + } + + var failures []lint.Failure + onFailure := func(failure lint.Failure) { + failures = append(failures, failure) + } + + w := lintUnsecureURLSchemeRule{ + onFailure: onFailure, + } + + ast.Walk(w, file.AST) + return failures +} + +// Name returns the rule name. +func (*UnsecureURLSchemeRule) Name() string { + return "unsecure-url-scheme" +} + +type lintUnsecureURLSchemeRule struct { + onFailure func(lint.Failure) +} + +const schemeSeparator = "://" +const schemeHTTP = "http" +const schemeWS = "ws" +const urlPrefixHTTP = schemeHTTP + schemeSeparator +const urlPrefixWS = schemeWS + schemeSeparator +const lenURLPrefixHTTP = len(urlPrefixHTTP) +const lenURLPrefixWS = len(urlPrefixWS) + +func (w lintUnsecureURLSchemeRule) Visit(node ast.Node) ast.Visitor { + n, ok := node.(*ast.BasicLit) + if !ok || n.Kind != token.STRING { + return w // not a string literal + } + + value, _ := strconv.Unquote(n.Value) // n.Value has one of the following forms: "..." or `...` + + var scheme string + var lenURLPrefix int + switch { + case strings.HasPrefix(value, urlPrefixHTTP): + scheme = schemeHTTP + lenURLPrefix = lenURLPrefixHTTP + case strings.HasPrefix(value, urlPrefixWS): + scheme = schemeWS + lenURLPrefix = lenURLPrefixWS + default: + return nil // not an URL or not an unsecure one + } + + if len(value) <= lenURLPrefix { + return nil // there is no host part in the string + } + + if strings.Contains(value, "localhost") || strings.Contains(value, "127.0.0.1") || strings.Contains(value, "0.0.0.0") || strings.Contains(value, "//::") { + return nil // do not fail on local URL + } + + w.onFailure(lint.Failure{ + Confidence: 1, + Failure: fmt.Sprintf("prefer secure protocol %s over %s in %s", scheme+"s", scheme, n.Value), + Node: n, + }) + + return nil +} diff --git a/test/unsecure_url_scheme_test.go b/test/unsecure_url_scheme_test.go new file mode 100644 index 0000000..b95ee3b --- /dev/null +++ b/test/unsecure_url_scheme_test.go @@ -0,0 +1,12 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/lint" + "github.com/mgechev/revive/rule" +) + +func TestUnsecureURLScheme(t *testing.T) { + testRule(t, "unsecure_url_scheme", &rule.UnsecureURLSchemeRule{}, &lint.RuleConfig{}) +} diff --git a/testdata/unsecure_url_scheme.go b/testdata/unsecure_url_scheme.go new file mode 100644 index 0000000..4e1a0ae --- /dev/null +++ b/testdata/unsecure_url_scheme.go @@ -0,0 +1,52 @@ +package fixtures + +import ( + "fmt" + "net/http" +) + +var url = "http://example.com" // MATCH /prefer secure protocol https over http in "http://example.com"/ + +const prefix = "http://" +const urlPattern = "http://%s:%d" // MATCH /prefer secure protocol https over http in "http://%s:%d"/ + +var wsURL = "ws://example.com" // MATCH /prefer secure protocol wss over ws in "ws://example.com"/ + +const wsPrefix = "ws://" +const wsURLPattern = "ws://%s" // MATCH /prefer secure protocol wss over ws in "ws://%s"/ + +func unsecureURLScheme() { + _ = fmt.Sprintf("http://%s", ipPort) // MATCH /prefer secure protocol https over http in "http://%s"/ + _ = fmt.Sprintf("http://%s/echo?msg=%s", ipPort, msg) // MATCH /prefer secure protocol https over http in "http://%s/echo?msg=%s"/ + _ = "http://::1" + _ = "http://::/" + http.Get("http://json-schema.org/draft-04/schema#/properties/") // MATCH /prefer secure protocol https over http in "http://json-schema.org/draft-04/schema#/properties/"/ + + _ = fmt.Sprintf("ws://%s", ipPort) // MATCH /prefer secure protocol wss over ws in "ws://%s"/ + _ = fmt.Sprintf("ws://%s/echo?msg=%s", ipPort, msg) // MATCH /prefer secure protocol wss over ws in "ws://%s/echo?msg=%s"/ + _ = "ws://::1" + _ = "ws://::/" + http.Get("ws://json-schema.org/draft-04/schema#/properties/") // MATCH /prefer secure protocol wss over ws in "ws://json-schema.org/draft-04/schema#/properties/"/ + + // Must not fail + println("http://localhost:8080", "http://0.0.0.0:8080") + if "http://127.0.0.1:80" == url { + } + + println("ws://localhost", "ws://0.0.0.0") + if "ws://127.0.0.1" == url { + } + + _ = fmt.Sprintf("wss://%s", ipPort) + _ = fmt.Sprintf("wss://%s/echo?msg=%s", ipPort, msg) + _ = "wss://::1" + _ = "wss://::/" + + _ = fmt.Sprintf("https://%s", ipPort) + _ = fmt.Sprintf("https://%s/echo?msg=%s", ipPort, msg) + _ = "https://::1" + _ = "https://::/" + http.Get("https://json-schema.org/draft-04/schema#/properties/") + _ = "http://" + "http:/" + "http" + _ = "ws://" + "ws:/" + "ws" +}