diff --git a/rules/go/lang/log_output_neutralization.yml b/rules/go/lang/log_output_neutralization.yml index da0287ec3..c239d89c9 100644 --- a/rules/go/lang/log_output_neutralization.yml +++ b/rules/go/lang/log_output_neutralization.yml @@ -14,10 +14,13 @@ patterns: - variable: INPUT detection: go_lang_log_output_neutralization_input - pattern: | - $.$.$($); + $.$().$($); filters: - - variable: ZEROLOG - detection: go_lang_log_output_neutralization_zerolog + - either: + - variable: CALLER + detection: go_lang_log_output_neutralization_logger + - variable: CALLER + detection: go_lang_log_output_neutralization_zerolog - variable: EVENT regex: \A(Info|Debug|Error|Trace|Fatal|Panic|Warn)\z - variable: METHOD @@ -61,6 +64,12 @@ auxiliary: patterns: - log.New(); - log.Default(); + - log + - import $"log" + - | + import ( + $"log" + ) - id: go_lang_log_output_neutralization_zerolog patterns: - import $"github.com/rs/zerolog" diff --git a/rules/go/lang/logger.yml b/rules/go/lang/logger.yml index f6ccd94c8..8f9877823 100644 --- a/rules/go/lang/logger.yml +++ b/rules/go/lang/logger.yml @@ -1,19 +1,113 @@ imports: - go_shared_lang_datatype patterns: - - pattern: log.$().$($) + - pattern: | + $.$($); filters: - - variable: LOGLEVEL - values: - - Error - - Debug + - either: + - variable: CALLER + detection: go_lang_logger_log + - variable: CALLER + detection: go_lang_logger_zerolog + - variable: METHOD + regex: \A(Fatal|Panic|Print)(f|ln)?\z + - variable: DATA_TYPE + detection: go_shared_lang_datatype + scope: result + - pattern: | + $.$().$($); + filters: + - either: + - variable: CALLER + detection: go_lang_logger_log + - variable: CALLER + detection: go_lang_logger_zerolog + - variable: EVENT + regex: \A(Info|Debug|Error|Trace|Fatal|Panic|Warn)\z - variable: METHOD values: - - Msgf - Msg + - Msgf + - variable: DATA_TYPE + detection: go_shared_lang_datatype + scope: result + - pattern: | + $.$($); + filters: + - either: + - variable: CALLER + detection: go_lang_logger_logrus + - variable: CALLER + detection: go_lang_logger_seelog + - variable: METHOD + regex: \A(WithFields\.)?(Info|Debug|Error|Trace|Fatal|Panic|Warn)\z + - variable: DATA_TYPE + detection: go_shared_lang_datatype + scope: result + - pattern: | + $.$($); + filters: + - variable: CALLER + detection: go_lang_logger_glog + - variable: METHOD + regex: \A(Info|Warning|Error|Fatal)(Contex)?(Depth)?(f)?\z + - variable: DATA_TYPE + detection: go_shared_lang_datatype + scope: result + - pattern: | + $.$($); + filters: + - variable: ZAP + detection: go_lang_logger_zap + - variable: METHOD + regex: \A(WithFields\.)?(Info|Log|Error|Fatal|DPanic|Warn)\z - variable: DATA_TYPE detection: go_shared_lang_datatype scope: result +auxiliary: + - id: go_lang_logger_log + patterns: + - log + - log.New(); + - log.Default(); + - import $"log" + - | + import ( + $"log" + ) + - id: go_lang_logger_zerolog + patterns: + - import $"github.com/rs/zerolog" + - | + import ( + $"github.com/rs/zerolog" + ) + - id: go_lang_logger_logrus + patterns: + - logrus.New(); + - import $"github.com/sirupsen/logrus" + - | + import ( + $"github.com/sirupsen/logrus" + ) + - id: go_lang_logger_zap + patterns: + - zap.$<_>().Sugar() + - zap.$<_>() + - id: go_lang_logger_seelog + patterns: + - import $"github.com/cihub/seelog" + - | + import ( + $"github.com/cihub/seelog" + ) + - id: go_lang_logger_glog + patterns: + - import $"github.com/golang/glog" + - | + import ( + $"github.com/golang/glog" + ) languages: - go skip_data_types: diff --git a/rules/go/lang/logger_leak.yml b/rules/go/lang/logger_leak.yml new file mode 100644 index 000000000..6f9e28d2f --- /dev/null +++ b/rules/go/lang/logger_leak.yml @@ -0,0 +1,157 @@ +imports: + - go_shared_lang_datatype +patterns: + - pattern: | + $.$($<...>$$<...>) + filters: + - either: + - variable: CALLER + detection: go_lang_logger_leak_logger + - variable: CALLER + detection: go_lang_logger_leak_zerolog + - variable: METHOD + regex: \A(Fatal|Panic|Print)(f|ln)?\z + - not: + variable: INPUT + detection: string_literal + scope: cursor + - not: + variable: INPUT + detection: go_shared_lang_datatype + scope: result + - pattern: | + $.$().$($<...>$$<...>); + filters: + - either: + - variable: CALLER + detection: go_lang_logger_leak_logger + - variable: CALLER + detection: go_lang_logger_leak_zerolog + - variable: EVENT + regex: \A(Info|Debug|Error|Trace|Fatal|Panic|Warn)\z + - variable: METHOD + values: + - Msg + - Msgf + - not: + variable: INPUT + detection: string_literal + scope: cursor + - not: + variable: INPUT + detection: go_shared_lang_datatype + scope: result + - pattern: | + $.$($<...>$$<...>); + filters: + - either: + - variable: CALLER + detection: go_lang_logger_leak_logrus + - variable: CALLER + detection: go_lang_logger_leak_seelog + - variable: METHOD + regex: \A(WithFields\.)?(Info|Debug|Error|Trace|Fatal|Panic|Warn)\z + - not: + variable: INPUT + detection: string_literal + scope: cursor + - not: + variable: INPUT + detection: go_shared_lang_datatype + scope: result + - pattern: | + $.$($<...>$$<...>); + filters: + - variable: CALLER + detection: go_lang_logger_leak_glog + - variable: METHOD + regex: \A(Info|Warning|Error|Fatal)(Contex)?(Depth)?(f)?\z + - not: + variable: INPUT + detection: string_literal + scope: cursor + - not: + variable: INPUT + detection: go_shared_lang_datatype + scope: result + - pattern: | + $.$($<...>$$<...>); + filters: + - variable: ZAP + detection: go_lang_logger_leak_zap + - variable: METHOD + regex: \A(WithFields\.)?(Info|Log|Error|Fatal|DPanic|Warn)\z + - not: + variable: INPUT + detection: string_literal + scope: cursor + - not: + variable: INPUT + detection: go_shared_lang_datatype + scope: result +auxiliary: + - id: go_lang_logger_leak_logger + patterns: + - log + - log.New(); + - log.Default(); + - import $"log" + - | + import ( + $"log" + ) + - id: go_lang_logger_leak_zerolog + patterns: + - import $"github.com/rs/zerolog" + - | + import ( + $"github.com/rs/zerolog" + ) + - id: go_lang_logger_leak_logrus + patterns: + - logrus.New(); + - import $"github.com/sirupsen/logrus" + - | + import ( + $"github.com/sirupsen/logrus" + ) + - id: go_lang_logger_leak_zap + patterns: + - zap.$<_>().Sugar() + - zap.$<_>() + - id: go_lang_logger_leak_seelog + patterns: + - import $"github.com/cihub/seelog" + - | + import ( + $"github.com/cihub/seelog" + ) + - id: go_lang_logger_leak_glog + patterns: + - import $"github.com/golang/glog" + - | + import ( + $"github.com/golang/glog" + ) +languages: + - go +severity: warning +metadata: + description: "Leakage of information in logger message" + remediation_message: | + ## Description + + Leaking data to loggers is a common cause of data leaks and can lead to data breaches. This rule looks for instances of dynamic data or variables sent to loggers. + + ## Remediations + + ❌ Avoid using variables or dynamic data in logger messages: + + ```go + logger.info(f"User is: '{user.email}'") + ``` + cwe_id: + - 532 + id: go_lang_logger_leak + documentation_url: https://docs.bearer.com/reference/rules/go_lang_logger_leak + cloud_code_suggestions: true diff --git a/tests/go/lang/log_output_neutralization/testdata/main.go b/tests/go/lang/log_output_neutralization/testdata/main.go index 58432e587..4f923c843 100644 --- a/tests/go/lang/log_output_neutralization/testdata/main.go +++ b/tests/go/lang/log_output_neutralization/testdata/main.go @@ -27,7 +27,7 @@ func bad() { func bad2() { // bearer:expected go_lang_log_output_neutralization - zerolog.Info.Msg(os.Args[0]) + zerolog.Info().Msg(os.Args[0]) // bearer:expected go_lang_log_output_neutralization zerolog.Print(os.Args[0]) } diff --git a/tests/go/lang/logger/test.js b/tests/go/lang/logger/test.js index 4dba89945..1a6b29430 100644 --- a/tests/go/lang/logger/test.js +++ b/tests/go/lang/logger/test.js @@ -1,30 +1,15 @@ -const { - createNewInvoker, - getEnvironment, -} = require("../../../helper.js") +const { createNewInvoker, getEnvironment } = require("../../../helper.js") const { ruleId, ruleFile, testBase } = getEnvironment(__dirname) describe(ruleId, () => { const invoke = createNewInvoker(ruleId, ruleFile, testBase) - - test("bad", () => { - const testCase = "bad.go" + test("logger", () => { + const testCase = "main.go" - const results = invoke(testCase) + const results = invoke(testCase) - expect(results.Missing).toEqual([]) - expect(results.Extra).toEqual([]) - }) - - - test("ok", () => { - const testCase = "ok.go" - - const results = invoke(testCase) - - expect(results.Missing).toEqual([]) - expect(results.Extra).toEqual([]) - }) - -}) \ No newline at end of file + expect(results.Missing).toEqual([]) + expect(results.Extra).toEqual([]) + }) +}) diff --git a/tests/go/lang/logger/testdata/main.go b/tests/go/lang/logger/testdata/main.go new file mode 100644 index 000000000..7df693c83 --- /dev/null +++ b/tests/go/lang/logger/testdata/main.go @@ -0,0 +1,145 @@ +package go_lang_logger + +import ( + seelog "github.com/cihub/seelog" + glog "github.com/golang/glog" + "github.com/rs/zerolog" + zerolog "github.com/rs/zerolog" + "github.com/rs/zerolog/log" + logrus "github.com/sirupsen/logrus" + "go.uber.org/zap" +) + +type User struct { + Name string + Uuid string + Gender string +} + +func (x User) FullName() (string, error) { + return "[" + x.Gender + "] " + x.Name, nil +} + +func bad() { + user := User{ + Uuid: "123", + Name: "foo", + Gender: "", + } + + name := user.Name + other, _ := user.FullName() + + // bearer:expected go_lang_logger + zerolog.Error().Msg(name) + // bearer:expected go_lang_logger + zerolog.Error().Msg(other) +} + +func bad2() { + user := User{ + Uuid: "123", + Name: "foo", + Gender: "", + } + + name := user.Name + other, _ := user.FullName() + + // bearer:expected go_lang_logger + zerolog.Info().Msg(name) + // bearer:expected go_lang_logger + zerolog.Print(other) +} + +func bad3() { + user := User{ + Uuid: "123", + Name: "foo", + Gender: "", + } + + other, _ := user.FullName() + + // bearer:expected go_lang_logger + logrus.WithFields().Info(user.Name) + + log := logrus.New() + // bearer:expected go_lang_logger + log.Error(other) +} + +func bad4() { + user := User{ + Uuid: "123", + Name: "foo", + Gender: "", + } + + name := user.Name + other, _ := user.FullName() + + var cfg zap.Config + logger := zap.Must(cfg.Build()) + defer logger.Sync() + + // bearer:expected go_lang_logger + logger.Info(name) + logger.Printf("Args: %s", other) +} + +func bad5() { + user := User{ + Uuid: "123", + Name: "foo", + Gender: "", + } + + name := user.Name + + // bearer:expected go_lang_logger + seelog.Trace(name) +} + +func bad6() { + user := User{ + Uuid: "123", + Name: "foo", + Gender: "", + } + + other, _ := user.FullName() + + // bearer:expected go_lang_logger + glog.Fatalf(other) +} + +func ok() { + user := User{ + Uuid: "123", + Name: "foo", + Gender: "", + } + + uuid := user.Uuid + + zerolog.Error().Msg(uuid) +} + +func main() { + user := User{ + Uuid: "123", + Name: "foo", + Gender: nil, + } + + name := user.Name + other, _ := user.FullName() + + // bearer:expected go_lang_logger + log.Error().Msg(name) // expect detection + // bearer:expected go_lang_logger + log.Error().Msg(other) // expect detection + // bearer:expected go_lang_logger + log.Error().Msg(user) // expect detection +} diff --git a/tests/go/lang/logger/testdata/ok.go b/tests/go/lang/logger/testdata/ok.go deleted file mode 100644 index d0a978200..000000000 --- a/tests/go/lang/logger/testdata/ok.go +++ /dev/null @@ -1,27 +0,0 @@ -package main3 - -import "github.com/rs/zerolog/log" - -// var a, b string - -type User struct { - Name string - Uuid string - Gender string -} - -func (x User) FullName() (string, error) { - return "[" + x.Gender + "] " + x.Name, nil -} - -func main() { - user := User{ - Uuid: "123", - Name: "foo", - Gender: nil, - } - - uuid := user.Uuid - - log.Error().Msg(uuid) // ok -} diff --git a/tests/go/lang/logger_leak/test.js b/tests/go/lang/logger_leak/test.js new file mode 100644 index 000000000..5a5e66118 --- /dev/null +++ b/tests/go/lang/logger_leak/test.js @@ -0,0 +1,18 @@ +const { + createNewInvoker, + getEnvironment, +} = require("../../../helper.js") +const { ruleId, ruleFile, testBase } = getEnvironment(__dirname) + +describe(ruleId, () => { + const invoke = createNewInvoker(ruleId, ruleFile, testBase) + + test("logger_leak", () => { + const testCase = "main.go" + + const results = invoke(testCase) + + expect(results.Missing).toEqual([]) + expect(results.Extra).toEqual([]) + }) +}) \ No newline at end of file diff --git a/tests/go/lang/logger_leak/testdata/main.go b/tests/go/lang/logger_leak/testdata/main.go new file mode 100644 index 000000000..3ac8c7211 --- /dev/null +++ b/tests/go/lang/logger_leak/testdata/main.go @@ -0,0 +1,72 @@ +package go_lang_logger_leak + +import ( + seelog "github.com/cihub/seelog" + glog "github.com/golang/glog" + zerolog "github.com/rs/zerolog" + logrus "github.com/sirupsen/logrus" + "go.uber.org/zap" +) + +func bad(Data any) { + // bearer:expected go_lang_logger_leak + zerolog.Error().Msg(Data) + // bearer:expected go_lang_logger_leak + zerolog.Error().Msgf("Some data: %#v", Data) +} + +func bad2(Data any) { + // bearer:expected go_lang_logger_leak + zerolog.Info().Msg(Data) + // bearer:expected go_lang_logger_leak + zerolog.Printf("Some data: %#v", Data) +} + +func bad3(Data any) { + // bearer:expected go_lang_logger_leak + logrus.WithFields().Info(Data) + + log := logrus.New() + // bearer:expected go_lang_logger_leak + log.Error("Some data: %#v", Data) +} + +func bad4(Data any) { + var cfg zap.Config + logger := zap.Must(cfg.Build()) + defer logger.Sync() + + // bearer:expected go_lang_logger_leak + logger.Info(Data) + logger.Printf("Args: %s", Data) +} + +func bad5(Data any) { + + // bearer:expected go_lang_logger_leak + seelog.Trace(Data) +} + +func bad6(Data any) { + // bearer:expected go_lang_logger_leak + glog.Fatalf("Some data: %#v", Data) +} + +type User struct { + Name string + Uuid string + Gender string +} + +func ok() { + user := User{ + Uuid: "123", + Name: "foo", + Gender: "", + } + name := user.Name + + zerolog.Error().Msg("Some string message") + // caught by another rule + zerolog.Error().Msg(name) +}