Skip to content

Enhance sast-rules lgpl\javascript\eval\rule-server-side-template-injection.yml

Background and Rationale behind this Work

As per https://gitlab.com/gitlab-org/gitlab/-/issues/425704 and https://gitlab.com/gitlab-org/gitlab/-/issues/425704 we are continuously working towards improving the coverage and efficacy of our SAST rules.

Desired Change

  1. Create a real world app or real world app test case demonstrating the flaw/issues.
  2. Assign a VR member as a reviewer of real world app linking back to the rule
  3. Determine if the patterns can be enhanced
  4. Determine if the description text can be enhanced. Prefer to have code examples demonstrating secure usage as well as links to OWASP or other authoritative resources. (See other languages' rules for examples of secure code)
  5. Determine if tests need to be updated with changes to patterns
  6. Add OWASP 2017 2021 mappings to the metadata where applicable

Implementation Plan

Assesment

If Creating a New Rule

  • Determine whether the rule in question currently exists or ever existed in our GitLab SAST Rules.
  • If it existed but was removed or merged into another rule, e.g. due to generating a high number of false positives, document said context in the comments and work to avoid the same problems with this new rule.

If Embedding or Adapting an External Rule

Determine whether the test code or minimal runnable example (MRE) accompanying the rule to be embedded or adapted:

  • Is realistic in its portrayal of the problem, e.g. test code for a Django rule crammed within a single Python file and containing no views or models isn't realistic.
  • Contains a sufficient number of true positive(TP) testcases(# ruleid: my-rule) and variations. In the case of Python, for example, rules and tests considering foo("bar") but not for foo(f"{bar}") nor foo("%s" % bar) are most likely insufficient.
  • Contains a sufficient number of true negative(TN) testcases(# okruleid: my-rule) and variations.
  • Determine whether the GitLab SAST ruleset detects instances of the vulnerability in the acompanying MRE and if there is room for improvement regarding when compared to the external rule (e.g. semgrep --test --config sast-rules/gitlab-rule.yml mre/ vs semgrep --test --config external-rule.yml mre/ ).

If Enhancing an already Exisiting Rule

Proceed to the next section.

Classification

  • In order to keep track of the ruleset evolution and also maintain proper licensing, classify the work in this issue and justify your decision:

If a new rule will be created from scratch due to poor performance of externally sourced rules, label this issue as SAST::RulesetAddition

If an existing rule will be enhanced, label this issue as SAST::RulesetEnhancement

If an equivalent rule doesn't exist already but embedding its externally sourced counterpart(s) improves our SAST Ruleset, label this issue as SAST::RulesetInclusion

If an equivalent rule exists already but can be improved based on external rules, label this issue as SAST::RulesetAdaptation

If the addition, inclusion or adaptation of a rule addressing the Desired Change in this issue adds no value to our SAST Ruleset, label this issue as SAST::RulesetSkip

Preparation

ℹ Think out loud as well as document your research and design decisions; feel free to do so at the end of this issue description or in individual comments.

Implementation

  • Create a simple but realistic Minimal Runnable Example (MRE), in e.g. mre/, which captures, initially, the most representative variant of the problem to be addressed by the rule.
  • Create a Semgrep rule, e.g. my-rule.yml, that correctly identifies the vulnerabilities present in your mre/ so far and place it in the language, framework and variant folder most suited for it, e.g. python/django/security/injection/sql.
  • For each instance of the problem to be detected (# ruleid: my-rule) or to be ignored (# okruleid: my-rule) within your MRE, add two comments above each occurrence:
# rationale: <description of the vulnerability or variant>
# ruleid: <java_some_rule_id>
  • One comment should explain the rationale behind the scenario and test-case, its pitfalls and anything you deem relevant, e.g. # this test case is necessary because string interpolations are handled in a different manner when compared to normal string literals
  • The second comment should follow Semgrep's rule testing guidelines, making it clear whether the line immediately following said comment represents a true positive ( # ruleid: my-rule ), a true negative ( # ok: my-rule ) or a false negative to be addressed in the future (# todoruleid: my-rule)
  • [] Enhance and extend your MRE as well as its associated rule with other important variants and corner cases of the same problem according to your prioritized list of scenarios. Make sure to add variations which are syntactically and semantically different enough to warrant doing so and which make both your rule and MRE more robust.
  • Continuously test your rule on your MRE ( semgrep scan --config my-rule.yml mre/ ) and update it as your MRE becomes more complex while also continuing to add comments and Semgrep test annotations to help verifying the desired coverage.
  • Once you are satisfied with the level of detail in your MRE as well as the coverage of your rule and its test results, distill your mre/ into a unit test file(e.g. rule.py) or series of files (test/rule.py) with the most representative test cases and place them into the same folder as your rule within the sast-rules project.

Merge the MRE

  • Clone our Real World Test Projects and extend it with your MRE demonstrating the problem. Alternatively, discuss the creation of a new folder or repository if none fits.
  • Push the changes to gitlab-org/security-products/tests/sast-rules-apps as a feature branch if you have access; otherwise push it to a personal fork of the project
  • Create a new MR and mention this issue in it so they are linked.
  • A member of the @gitlab-org/secure/vulnerability-research team will assign themselves as reviewer shortly, work with them to finalise and merge your work.

Merge the Rule

  • Push the changes to sast-rules as a feature branch to gitlab-org/security-products/sast-rules if you have access; otherwise push it to a personal fork of the project.
  • Create the MR and mention this issue in it so they are linked.
  • A member of the @gitlab-org/secure/vulnerability-research team will assign themselves as reviewer shortly, work with them to finalise and merge your work.
  • Find the latest sast-rules release MR and add a line to CHANGELOG.md detailing briefly the changes performed, their intent and the MR ID where this work was done.
## v2.X.X
- Adds/Changes/Updates new pattern to existing rule `java/xss/rule-XXX.yml` for Java that checks XXX for XXX (!<merge request id>) 
...

Workflow

Use the SAST Improvement Board to track the progress of your issues or find other SAST work to get involved with.

Use the workflowready for development, workflowin dev and workflowready for review labels to signal, respectively, that this issue is ready to be worked on, this issue is actively being worked on by the assignee or this issue and its associated MR require review before proceeding.