From c6ea106da3864b9a66355c47a31a9c948ca8bcba Mon Sep 17 00:00:00 2001 From: Ajin Abraham Date: Tue, 8 Jun 2021 21:31:53 -0700 Subject: [PATCH] support njsscan-ignore for templates (#64) * support njsscan-ignore for templates (#64) * depricate `ignore:` --- njsscan/__init__.py | 2 +- njsscan/njsscan.py | 46 +++++++++++++- njsscan/patcher.py | 31 +++++++++ .../rules/pattern_matcher/template_rules.yaml | 63 +++++++++++-------- tests/assets/dot_njsscan/ignore_template.hbs | 3 + tests/assets/dot_njsscan/lorem_scan.js | 2 +- 6 files changed, 116 insertions(+), 31 deletions(-) create mode 100644 njsscan/patcher.py create mode 100644 tests/assets/dot_njsscan/ignore_template.hbs diff --git a/njsscan/__init__.py b/njsscan/__init__.py index 5846d5d..5c2639c 100644 --- a/njsscan/__init__.py +++ b/njsscan/__init__.py @@ -6,7 +6,7 @@ __title__ = 'njsscan' __authors__ = 'Ajin Abraham' __copyright__ = f'Copyright {datetime.now().year} Ajin Abraham, OpenSecurity' -__version__ = '0.2.7' +__version__ = '0.2.8' __version_info__ = tuple(int(i) for i in __version__.split('.')) __all__ = [ '__title__', diff --git a/njsscan/njsscan.py b/njsscan/njsscan.py index 02e6093..cc52ea0 100644 --- a/njsscan/njsscan.py +++ b/njsscan/njsscan.py @@ -1,16 +1,21 @@ # -*- coding: utf_8 -*- """The nodejsscan cli: njsscan.""" +from linecache import getline + from libsast import Scanner +from libsast import standards from njsscan import settings from njsscan.utils import ( get_config, read_missing_controls, ) +from njsscan.patcher import patch_libsast class NJSScan: def __init__(self, paths, json, check_controls, config=False) -> None: + patch_libsast() conf = get_config(paths, config) self.check_controls = check_controls self.options = { @@ -30,6 +35,7 @@ def __init__(self, paths, json, check_controls, config=False) -> None: 'nodejs': {}, 'errors': [], } + self.standards = standards.get_standards() def scan(self) -> dict: """Start Scan.""" @@ -60,6 +66,17 @@ def missing_controls(self, result): else: continue + def expand_mappings(self, meta): + """Expand libsast standard mappings.""" + meta_keys = meta['metadata'].keys() + for mkey in meta_keys: + if mkey not in self.standards.keys(): + continue + to_expand = meta['metadata'][mkey] + expanded = self.standards[mkey].get(to_expand) + if expanded: + meta['metadata'][mkey] = expanded + def format_sgrep(self, sgrep_output): """Remove metavars from sgrep output.""" self.result['errors'] = sgrep_output['errors'] @@ -71,6 +88,9 @@ def format_sgrep(self, sgrep_output): def format_matches(self, matcher_out): """Format Pattern Matcher output.""" + for rule_id in matcher_out: + # TODO Remove after standards is handled in libsast + self.expand_mappings(matcher_out[rule_id]) self.result['templates'] = matcher_out def post_ignore_rules(self): @@ -81,6 +101,15 @@ def post_ignore_rules(self): if rule_id in self.result['templates']: del self.result['templates'][rule_id] + def suppress_pm_comments(self, obj, rule_id): + """Suppress pattern matcher.""" + file_path = obj['file_path'] + lines = obj['match_lines'] + match_line = getline(file_path, lines[0]) + if 'njsscan-ignore:' in match_line and rule_id in match_line: + return True + return False + def post_ignore_files(self): """Ignore file by rule.""" del_keys = set() @@ -91,8 +120,19 @@ def post_ignore_files(self): tmp_files = files.copy() for file in files: mstr = file.get('match_string') - cmt = 'ignore:' in mstr or 'njsscan-ignore:' in mstr - if cmt and rule_id in mstr: + if 'njsscan-ignore:' in mstr and rule_id in mstr: + tmp_files.remove(file) + if len(tmp_files) == 0: + del_keys.add(rule_id) + details['files'] = tmp_files + for rule_id, details in self.result['templates'].items(): + files = details.get('files') + if not files: + continue + tmp_files = files.copy() + for file in files: + mstr = file.get('match_string') + if self.suppress_pm_comments(file, rule_id): tmp_files.remove(file) if len(tmp_files) == 0: del_keys.add(rule_id) @@ -101,3 +141,5 @@ def post_ignore_files(self): for rid in del_keys: if rid in self.result['nodejs']: del self.result['nodejs'][rid] + if rid in self.result['templates']: + del self.result['templates'][rid] diff --git a/njsscan/patcher.py b/njsscan/patcher.py new file mode 100644 index 0000000..05f36e7 --- /dev/null +++ b/njsscan/patcher.py @@ -0,0 +1,31 @@ +"""Libsast Patcher for supporting rules with metadata.""" +from copy import deepcopy + +from libsast.core_matcher.pattern_matcher import PatternMatcher + + +def add_finding(self, file_path, rule, matches): + """Add Code Analysis Findings.""" + for match in matches: + crule = deepcopy(rule) + file_details = { + 'file_path': file_path.as_posix(), + 'match_string': match[0], + 'match_position': match[1], + 'match_lines': match[2], + } + if rule['id'] in self.findings: + self.findings[rule['id']]['files'].append(file_details) + else: + metadata = crule['metadata'] + metadata['description'] = crule['message'] + metadata['severity'] = crule['severity'] + self.findings[rule['id']] = { + 'files': [file_details], + 'metadata': metadata, + } + + +def patch_libsast(): + """Patch Libsast.""" + PatternMatcher.add_finding = add_finding diff --git a/njsscan/rules/pattern_matcher/template_rules.yaml b/njsscan/rules/pattern_matcher/template_rules.yaml index 2fb6071..6f3d1a7 100644 --- a/njsscan/rules/pattern_matcher/template_rules.yaml +++ b/njsscan/rules/pattern_matcher/template_rules.yaml @@ -1,91 +1,100 @@ --- - id: handlebar_mustache_template - description: The Handlebar.js/Mustache.js template has an unescaped variable. Untrusted + message: The Handlebar.js/Mustache.js template has an unescaped variable. Untrusted user input passed to this variable results in Cross Site Scripting (XSS). type: Regex pattern: '{{{(?!.*body).+}}}|{{[ ]*&[\w]+.*}}' severity: ERROR input_case: exact - cwe: "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" - owasp: "A1: Injection" + metadata: + cwe: cwe-79 + owasp: "A1: Injection" - id: dust_template - description: The Dust.js template has an unescaped variable. Untrusted user input + message: The Dust.js template has an unescaped variable. Untrusted user input passed to this variable results in Cross Site Scripting (XSS) type: Regex pattern: '{.+\|[ ]*s[ ]*}[^}]' severity: ERROR input_case: exact - cwe: "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" - owasp: "A1: Injection" + metadata: + cwe: cwe-79 + owasp: "A1: Injection" - id: pug_jade_template - description: The Pug.js/Jade.js template has an unescaped variable. Untrusted user + message: The Pug.js/Jade.js template has an unescaped variable. Untrusted user input passed to this variable results in Cross Site Scripting (XSS). type: Regex pattern: '!{.+}' severity: ERROR input_case: exact - cwe: "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" - owasp: "A1: Injection" + metadata: + cwe: cwe-79 + owasp: "A1: Injection" - id: ejs_ect_template - description: The EJS/ECT template has an unescaped variable. Untrusted user input + message: The EJS/ECT template has an unescaped variable. Untrusted user input passed to this variable results in Cross Site Scripting (XSS). type: Regex pattern: <%-(?![ ]*include\().*%> severity: ERROR input_case: exact - cwe: "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" - owasp: "A1: Injection" + metadata: + cwe: cwe-79 + owasp: "A1: Injection" - id: vue_template - description: The Vue.js template has an unescaped variable. Untrusted user input + message: The Vue.js template has an unescaped variable. Untrusted user input passed to this variable results in Cross Site Scripting (XSS). type: Regex pattern: v-html=[\'|"].+[\'|"] severity: ERROR input_case: exact - cwe: "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" - owasp: "A1: Injection" + metadata: + cwe: cwe-79 + owasp: "A1: Injection" - id: underscore_template - description: The Underscore unescape function with untrusted user input results + message: The Underscore unescape function with untrusted user input results in Cross Site Scripting (XSS). type: Regex pattern: '_.unescape\(.+\)' severity: ERROR input_case: exact - cwe: "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" - owasp: "A1: Injection" + metadata: + cwe: cwe-79 + owasp: "A1: Injection" - id: squirrelly_template - description: The Squirrelly.js template has an unescaped variable. Untrusted user input + message: The Squirrelly.js template has an unescaped variable. Untrusted user input passed to this variable results in Cross Site Scripting (XSS) type: Regex pattern: '{{.+\|.*safe.*}}' severity: ERROR input_case: exact - cwe: "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" - owasp: "A1: Injection" + metadata: + cwe: cwe-79 + owasp: "A1: Injection" - id: electronjs_node_integration - description: Node integration exposes node.js APIs to the electron app and this + message: Node integration exposes node.js APIs to the electron app and this can introduce remote code execution vulnerabilities to the application if the app is vulnerable to Cross Site Scripting (XSS). type: Regex pattern: + window.MY_VAR = "{{{val}}}"; // njsscan-ignore: handlebar_mustache_template + \ No newline at end of file diff --git a/tests/assets/dot_njsscan/lorem_scan.js b/tests/assets/dot_njsscan/lorem_scan.js index bb5896e..b7be5e7 100644 --- a/tests/assets/dot_njsscan/lorem_scan.js +++ b/tests/assets/dot_njsscan/lorem_scan.js @@ -48,7 +48,7 @@ app.get('/', function (req, res) { app.get('/some/path', function (req, res) { var target = req.param("target"); // BAD: sanitization doesn't apply here - res.redirect(target); //ignore: express_open_redirect + res.redirect(target); //njsscan-ignore: express_open_redirect }); app.listen(8888);