Skip to content

Commit

Permalink
feat(express rule): detect missing usage within same file (#711)
Browse files Browse the repository at this point in the history
* feat(express rule): detect missing library within same file

* refactor: improve rule documentation for helmet

* feat(expressjs rule): add reduce fingerprint rule

* chore: update tests file with express

* chore: update trigger name to be closer to presence

* chore: increase timeout for integration tests
  • Loading branch information
cfabianski committed Mar 3, 2023
1 parent 85650e1 commit 7658ca0
Show file tree
Hide file tree
Showing 95 changed files with 863 additions and 435 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/integration_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
with:
go-version: 1.19
- name: Run tests
run: go test -v ./integration/... -p 8
timeout-minutes: 10
run: go test -v ./integration/... -p 5
timeout-minutes: 15
env:
USE_BINARY: true
2 changes: 1 addition & 1 deletion integration/rules/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (runner *Runner) runTest(t *testing.T, projectPath string) {

files, err := filelist.Discover(testDataPath, runner.config)
if err != nil {
t.Fatalf("failed to discover files: %e", err)
t.Fatalf("failed to discover files: %s", err)
}

if len(files) == 0 {
Expand Down
10 changes: 10 additions & 0 deletions integration/rules/javascript_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,13 @@ func TestJavascripPassportHardcodedSecret(t *testing.T) {
t.Parallel()
getRunner(t).runTest(t, javascriptRulesPath+"third_parties/passport_hardcoded_secret")
}

func TestJavascriptHelmetMissing(t *testing.T) {
t.Parallel()
getRunner(t).runTest(t, javascriptRulesPath+"express/helmet_missing")
}

func TestJavascriptReduceFingerprint(t *testing.T) {
t.Parallel()
getRunner(t).runTest(t, javascriptRulesPath+"express/reduce_fingerprint")
}
8 changes: 8 additions & 0 deletions new/detector/implementation/custom/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ func matchFilter(
evaluator types.Evaluator,
filter settings.PatternFilter,
) (*bool, []*types.Detection, error) {
if filter.None != nil {
match, _, err := matchFilter(result, evaluator, *filter.None)
if match == nil {
return boolPointer(true), nil, err
}
return nil, nil, err
}

if filter.Not != nil {
match, _, err := matchFilter(result, evaluator, *filter.Not)
if match == nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/classification/schema/internal/testhelper/testhelper.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,14 @@ func ExtractExpectedOutput(

val, err := os.ReadFile("././fixtures/" + lang + ".json")
if err != nil {
t.Errorf("error opening file %e", err)
t.Errorf("error opening file %s", err)
}

var input []Input
rawBytes := []byte(val)
err = json.Unmarshal(rawBytes, &input)
if err != nil {
t.Errorf("error unmarshalling JSON %e", err)
t.Errorf("error unmarshalling JSON %s", err)
}

for _, inputItem := range input {
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/artifact/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func NewRunner(ctx context.Context, scanSettings settings.Config) Runner {

scanID, err := buildScanID(scanSettings)
if err != nil {
log.Error().Msgf("failed to build scan id for caching %e", err)
log.Error().Msgf("failed to build scan id for caching %s", err)
}

path := os.TempDir() + "/bearer" + scanID
Expand Down
10 changes: 5 additions & 5 deletions pkg/commands/process/balancer/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (process *Process) StartProcess(task *workertype.ProcessRequest) error {
var err error
currentCommand, err := os.Executable()
if err != nil {
log.Fatal().Msgf("failed to get current command executable %e", err)
log.Fatal().Msgf("failed to get current command executable %s", err)
}

if process.isExternalWorker {
Expand Down Expand Up @@ -129,7 +129,7 @@ func (process *Process) WaitForOnline(task *workertype.ProcessRequest) error {

req, err := http.NewRequestWithContext(process.context, http.MethodPost, process.workerUrl+workertype.RouteStatus, bytes.NewBuffer(marshalledConfig))
if err != nil {
log.Debug().Msgf("%s %s failed to build status online request %e", process.uuid, process.workeruuid, err)
log.Debug().Msgf("%s %s failed to build status online request %s", process.uuid, process.workeruuid, err)
continue
}

Expand Down Expand Up @@ -170,20 +170,20 @@ func (process *Process) doTask(task *Task) {
go func() {
taskBytes, err := json.Marshal(task.Definition)
if err != nil {
log.Debug().Msgf("failed to marshall task %e", err)
log.Debug().Msgf("failed to marshall task %s", err)
return
}

req, err := http.NewRequestWithContext(process.context, http.MethodPost, process.workerUrl+workertype.RouteProcess, bytes.NewBuffer(taskBytes))
if err != nil {
log.Debug().Msgf("%s %s failed to build process request %e", process.uuid, process.workeruuid, err)
log.Debug().Msgf("%s %s failed to build process request %s", process.uuid, process.workeruuid, err)
return
}

resp, err := process.client.Do(req)

if err != nil {
log.Debug().Msgf("%s %s failed to do process request %e", process.uuid, process.workeruuid, err)
log.Debug().Msgf("%s %s failed to do process request %s", process.uuid, process.workeruuid, err)
return
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/commands/process/balancer/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func (worker *Worker) Start() {
shouldBreak = true
case response := <-worker.processErrored:
// add failed files to report
log.Debug().Msgf("worker %s got process error %e", worker.uuid, response.Error)
log.Debug().Msgf("worker %s got process error %s", worker.uuid, response.Error)
if worker.process != nil {
log.Debug().Msgf("process is not nil killing it")
worker.process.kill()
Expand All @@ -169,15 +169,15 @@ func (worker *Worker) Start() {
// ungzip report and add it to master file
f, err := os.Open(tmpReportFile)
if err != nil {
log.Error().Msgf("worker %s failed to open tmp report chunk file %e", worker.uuid, err)
log.Error().Msgf("worker %s failed to open tmp report chunk file %s", worker.uuid, err)
worker.complete(err)

break
}

reportBytes, err := io.ReadAll(f)
if err != nil {
log.Error().Msgf("worker %s failed to read tmp report chunk file %e", worker.uuid, err)
log.Error().Msgf("worker %s failed to read tmp report chunk file %s", worker.uuid, err)
worker.complete(err)
f.Close()
break
Expand All @@ -191,7 +191,7 @@ func (worker *Worker) Start() {

err := bar.Add(len(work))
if err != nil {
log.Error().Msgf("worker %s failed to write progress bar %e", worker.uuid, err)
log.Error().Msgf("worker %s failed to write progress bar %s", worker.uuid, err)
}

if shouldBreak {
Expand Down Expand Up @@ -297,7 +297,7 @@ func (worker *Worker) logError(reportFile *os.File, work []workertype.File, resp
for _, file := range work {
fileInfo, err := os.Stat(worker.task.Definition.Dir + "/" + file.FilePath)
if err != nil {
log.Debug().Msgf("worker %s failed to stat file %e", worker.uuid, err)
log.Debug().Msgf("worker %s failed to stat file %s", worker.uuid, err)
continue
}

Expand All @@ -312,7 +312,7 @@ func (worker *Worker) logError(reportFile *os.File, work []workertype.File, resp

err := jsonlines.Encode(reportFile, &errorsToAdd)
if err != nil {
log.Error().Msgf("worker %s failed to encode data line %e", worker.uuid, err)
log.Error().Msgf("worker %s failed to encode data line %s", worker.uuid, err)
}

}
30 changes: 30 additions & 0 deletions pkg/commands/process/settings/policies/risk_policy.rego
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,36 @@ presence_failures contains detector if {
detector.detector_id == input.rule.id
}

policy_failure contains item if {
input.rule.trigger == "absence"
some detector in input.dataflow.risks

detector.detector_id == input.rule.trigger_rule_on_presence_of
some init_location in detector.locations

x := {other | other := input.dataflow.risks[_]; other.detector_id == input.rule.id}
count(x) == 0

item := data.bearer.common.build_item(init_location)
}

policy_failure contains item if {
input.rule.trigger == "absence"
some detector in input.dataflow.risks

detector.detector_id == input.rule.trigger_rule_on_presence_of

some init_location in detector.locations
some other_detector in input.dataflow.risks

other_detector.detector_id == input.rule.id

x := {other_location | other_location := other_detector.locations[_]; init_location.filename == other_location.filename}
count(x) == 0

item := data.bearer.common.build_item(init_location)
}

local_data_types contains data_type if {
not input.rule.skip_data_types
not input.rule.only_data_types
Expand Down
39 changes: 20 additions & 19 deletions pkg/commands/process/settings/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,25 +178,26 @@ func buildRules(definitions map[string]RuleDefinition, enabledRules map[string]s
}

rules[id] = &Rule{
Id: id,
Type: ruleType,
AssociatedRecipe: definition.Metadata.AssociatedRecipe,
Trigger: definition.Trigger,
SkipDataTypes: definition.SkipDataTypes,
OnlyDataTypes: definition.OnlyDataTypes,
Severity: mapSeverityKeysToCategories(definition.Severity),
Description: definition.Metadata.Description,
RemediationMessage: definition.Metadata.RemediationMessage,
Stored: definition.Stored,
Detectors: definition.Detectors,
Processors: definition.Processors,
AutoEncrytPrefix: definition.AutoEncrytPrefix,
CWEIDs: definition.Metadata.CWEIDs,
Languages: definition.Languages,
ParamParenting: definition.ParamParenting,
Patterns: definition.Patterns,
DocumentationUrl: definition.Metadata.DocumentationUrl,
OmitParentContent: definition.OmitParentContent,
Id: id,
Type: ruleType,
AssociatedRecipe: definition.Metadata.AssociatedRecipe,
Trigger: definition.Trigger,
SkipDataTypes: definition.SkipDataTypes,
OnlyDataTypes: definition.OnlyDataTypes,
Severity: mapSeverityKeysToCategories(definition.Severity),
Description: definition.Metadata.Description,
RemediationMessage: definition.Metadata.RemediationMessage,
Stored: definition.Stored,
Detectors: definition.Detectors,
Processors: definition.Processors,
AutoEncrytPrefix: definition.AutoEncrytPrefix,
CWEIDs: definition.Metadata.CWEIDs,
Languages: definition.Languages,
ParamParenting: definition.ParamParenting,
Patterns: definition.Patterns,
DocumentationUrl: definition.Metadata.DocumentationUrl,
OmitParentContent: definition.OmitParentContent,
TriggerRuleOnPresenceOf: definition.TriggerRuleOnPresenceOf,
}

for _, auxiliaryDefinition := range definition.Auxiliary {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ low:
id: javascript_express_cross_site_scripting
description: Cross-site scripting (XSS) vulnerability detected.
documentation_url: https://docs.bearer.com/reference/rules/javascript_express_cross_site_scripting
line_number: 5
line_number: 9
filename: res_send_xss.js
parent_line_number: 5
parent_line_number: 9
parent_content: res.send("<p>" + req.body.customer.name + "</p>")
- rule:
cwe_ids:
- "79"
id: javascript_express_cross_site_scripting
description: Cross-site scripting (XSS) vulnerability detected.
documentation_url: https://docs.bearer.com/reference/rules/javascript_express_cross_site_scripting
line_number: 9
line_number: 13
filename: res_send_xss.js
parent_line_number: 9
parent_line_number: 13
parent_content: res.send("<p>" + req.body["user_id"] + "</p>")


Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ low:
id: javascript_express_cross_site_scripting
description: Cross-site scripting (XSS) vulnerability detected.
documentation_url: https://docs.bearer.com/reference/rules/javascript_express_cross_site_scripting
line_number: 6
line_number: 10
filename: res_write_xss.js
parent_line_number: 6
parent_line_number: 10
parent_content: res.write("<h3> Greetings " + customerName + "</h3>")


Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
const express = require("express");
const app = express();
const express = require("express")
var helmet = require("helmet")

var app = express()
app.use(helmet())
app.use(helmet.hidePoweredBy())

app.get("/good", (_, res) => {
return res.send("<p>hello world</p>")
})

app.get("/good-2", () => {
// don't match on req params within strings
return res.send({ success: false, text: `User ${req.params.user_id} not found` });
return res.send({
success: false,
text: `User ${req.params.user_id} not found`,
})
})

app.get("/good-3", () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
const express = require("express");
const app = express();
const express = require("express")
var helmet = require("helmet")

var app = express()
app.use(helmet())
app.use(helmet.hidePoweredBy())

app.get("/bad", (req, res) => {
res.send("<p>" + req.body.customer.name + "</p>")
})

app.get("/bad-2", (req, res) => {
res.send("<p>" + req.body["user_id"] + "</p>")
})
})
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
const express = require("express");
const app = express();
const express = require("express")
var helmet = require("helmet")

var app = express()
app.use(helmet())
app.use(helmet.hidePoweredBy())

app.get("/bad", (req, res) => {
var customerName = req.body.customer.name
res.write("<h3> Greetings " + customerName + "</h3>")
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ low:
id: express_default_cookie_config
description: Cookie with default config detected.
documentation_url: https://docs.bearer.com/reference/rules/express_default_cookie_config
line_number: 6
line_number: 11
filename: default_cookie_session_config.js
parent_line_number: 6
parent_line_number: 11
parent_content: |-
{
domain: "example.com",
Expand Down
Loading

0 comments on commit 7658ca0

Please sign in to comment.