Skip to content

ruby dangerous-exec rule did not report dangerous usages properly #3396

Open
@JazJas

Description

Describe the bug
The existing rule "semgrep-rules/blob/develop/ruby/lang/security/dangerous-exec.yaml" did not recognize dangerous usages properly. It could miss dangerous usage, and also incorrectly report a proper usage as dangerous.

To Reproduce
The sample code containing both dangerous and secure 'system' command usages (in total four cases) is given below:
1 class T
2 def initialize(inputs)
3 @input = inputs[1]
4 # Case 1, vulnerable usage of system command, reported, expected but don't believe this is because the dangerous syntax is correctly recognised.
5 system "echo #{@input}"
6 # Case 2, secure usage of system command, reported, unexpected
7 system "echo", @input
8 end
9 def process
10 # Case 3, vulnerable usage of system command, but not reported, unexpected
11 system "echo #{@input}"
12 # Case 4, secure usage of system command, not reported, although expected, but don't believe this is because the secure syntax is correctly recognised.
13 system "echo", @input
14 end
15 end
16
17 # Sample input with user-controlled data
18 inputs = ["input 1", "input 2; echo 'Malicious code executed'"]
19 p = T.new(inputs)
20 p.process

The output of running the script are:
$ ruby test.rb
input 2
Malicious code executed
input 2; echo 'Malicious code executed'
input 2
Malicious code executed
input 2; echo 'Malicious code executed'

These indicated that Case 1 and Case 3 are vulnerable to command injection. However, the scan results show that Case 1 and Case 2 are vulnerable to command injection:

$ semgrep --config "p/ruby" test.rb

┌─────────────┐
│ Scan Status │
└─────────────┘
Scanning 1 file (only git-tracked) with 65 Code rules:

CODE RULES
Scanning 1 file with 65 ruby rules.

SUPPLY CHAIN RULES

💎 Run semgrep ci to find dependency
vulnerabilities and advanced cross-file findings.

PROGRESS

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00

┌─────────────────┐
│ 2 Code Findings │
└─────────────────┘

test.rb
❯❱ ruby.lang.security.dangerous-exec.dangerous-exec
      Detected non-static command inside system. Audit the input to 'system'. If unverified user data can
      reach this call site, this is a code injection vulnerability. A malicious actor can inject a
      malicious script to execute arbitrary code.
      Details: https://sg.run/R8GY

        5┆ system "echo #{@input}"
        ⋮┆----------------------------------------
        7┆ system "echo", @input

┌──────────────┐
│ Scan Summary │
└──────────────┘

Ran 65 rules on 1 file: 2 findings.

Expected behavior
The dangerous usages of 'system' command at Lines 5 and 11 should be reported. On the other hand, the secure usages at Lines 7 and 13 should NOT be reported.

Priority
How important is this to you?

  • P0: blocking me from making progress
    There is no proper option to choose.
    I chose P0 because this bug impacts the reputation of semgrep. Due to this issue, I lack confidence in trusting the output of this rule, and potentially other rules as well.

Additional Context
According to my observations, it seems the rule might be incorrectly affected by the scope/context of variables presented in the 'system' command.

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions