Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore file not working with GH Action Security Scan #1724

Closed
ds-dustenharrison opened this issue Jul 19, 2022 · 8 comments
Closed

Ignore file not working with GH Action Security Scan #1724

ds-dustenharrison opened this issue Jul 19, 2022 · 8 comments

Comments

@ds-dustenharrison
Copy link

ds-dustenharrison commented Jul 19, 2022

Background

Brakeman version: 5.2.3
Rails version: 3.0.4
Ruby version: 7.0.3

Link to Rails application code: I am unable to supply due to it being in a private repo.

Issue

What problem are you seeing? Here is my command line in the GH Action.
brakeman -i ./config/brakeman.ignore --format sarif --output output.sarif.json .
I see in the logs it is referencing the ignore file when running:
........
Checks finished, collecting results...
Filtering warnings...
Notice: Using './config/brakeman.ignore' to filter warnings
Generating report...
Report saved in 'output.sarif.json'
Error: Process completed with exit code 3.
.......

However, it does not seem to be using the ignore file. For example, this is from the ignore file:
{
"ignored_warnings": [
{
"warning_type": "SSL Verification Bypass",
"warning_code": 71,
"fingerprint": "6360b930243f37f472df72c8a4b09641121b6c3d32d3f24b0ee1af609afc6908",
"check_name": "SSLVerify",
"message": "SSL certificate verification was bypassed",
"file": "app/lib/secure_http_client.rb",
"line": 14,
"link": "https://brakemanscanner.org/docs/warning_types/ssl_verification_bypass/",
"code": "self.verify_mode = OpenSSL::SSL::VERIFY_NONE",
"render_path": null,
"location": {
"type": "method",
"class": "SecureHttpClient",
"method": "initialize"
},
"user_input": null,
"confidence": "High",
"note": "SSL verification is skipped in local development only"
},

However, in the GH Code Scanning results it is reporting this issue:
app/lib/secure_http_client.rb:14
self.use_ssl = true
if Rails.env.development?
logger.debug 'Disabling Open SSL verification in development environment'
self.verify_mode = OpenSSL::SSL::VERIFY_NONE
SSL certificate verification was bypassed.
Brakeman
else
self.verify_mode = OpenSSL::SSL::VERIFY_PEER
self.verify_depth = 5

There are 4 entries in our ignore file, and we see all 4 in the findings uploaded to GH Security.

Other Error

Run Brakeman with --debug to see the full stack trace.

Stack trace:

I looked at the output using --debug and don't see a stack trace. It just adds quite a bit of more information about the scan. The output went from 1,320 to 9,414 lines.
@presidentbeef
Copy link
Owner

Hi @ds-dustenharrison - can you run with -f json and check the fingerprints match the ignore file? The fingerprint is actually the only value Brakeman uses to ignore warnings.

Or you can run with -I and check if all the warnings are being ignored. I'm just trying to see if the fingerprints match and/or if this is an issue specific to the SARIF format.

@ds-dustenharrison
Copy link
Author

ds-dustenharrison commented Jul 19, 2022

I added the -I as a start. Looks like a potential formatting issue for the ignore file. We use the ignore file today via circleci and wanted to use it in GH now.
Here is the content of the ignore file.

{
"ignored_warnings": [
{
"warning_type": "SSL Verification Bypass",
"warning_code": 71,
"fingerprint": "6360b930243f37f472df72c8a4b09641121b6c3d32d3f24b0ee1af609afc6908",
"check_name": "SSLVerify",
"message": "SSL certificate verification was bypassed",
"file": "app/lib/secure_http_client.rb",
"line": 14,
"link": "https://brakemanscanner.org/docs/warning_types/ssl_verification_bypass/",
"code": "self.verify_mode = OpenSSL::SSL::VERIFY_NONE",
"render_path": null,
"location": {
"type": "method",
"class": "SecureHttpClient",
"method": "initialize"
},
"user_input": null,
"confidence": "High",
"note": "SSL verification is skipped in local development only"
},
{
"warning_type": "Mass Assignment",
"warning_code": 105,
"fingerprint": "63d67de8200c0b8521ca878dbc98e7b0b22528d72e788dfca348aa7b0c9ae5a3",
"check_name": "PermitAttributes",
"message": "Potentially dangerous key allowed for mass assignment",
"file": "app/controllers/docusign_oauth_controller.rb",
"line": 137,
"link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
"code": "params.permit(:account_id, :envelope_id, :envelope_site)",
"render_path": null,
"location": {
"type": "method",
"class": "DocusignOauthController",
"method": "sso_params"
},
"user_input": ":account_id",
"confidence": "High",
"note": "DocuSign Account ID is not a secret and it's not used for mass assignment. User must successfully auth to DS and then account ID param is only used to help select a specific account they have access to if they have more than one."
},
{
"warning_type": "Redirect",
"warning_code": 18,
"fingerprint": "a97eacc3d7d92c0f3615fcbed6aba49044270963a4e774ee1799072afa1e8de5",
"check_name": "Redirect",
"message": "Possible unprotected redirect",
"file": "app/controllers/rooms_controller.rb",
"line": 155,
"link": "https://brakemanscanner.org/docs/warning_types/redirect/",
"code": "redirect_to(RoomToken.find_by_token(params[:id]).room.envelope.pages.where(:documents => ({ :identifier => params[:document] }), :page => params[:page]).first!.image_url)",
"render_path": null,
"location": {
"type": "method",
"class": "RoomsController",
"method": "pageimg"
},
"user_input": "RoomToken.find_by_token(params[:id]).room.envelope.pages.where(:documents => ({ :identifier => params[:document] }), :page => params[:page]).first!.image_url",
"confidence": "High",
"note": ""
},
{
"warning_type": "Redirect",
"warning_code": 18,
"fingerprint": "f7b353359cdeb112fe2c758704c8210f530555a06ba2847e35fe00aec03c83bc",
"check_name": "Redirect",
"message": "Possible unprotected redirect",
"file": "app/controllers/rooms_controller.rb",
"line": 85,
"link": "https://brakemanscanner.org/docs/warning_types/redirect/",
"code": "redirect_to(UserEvent.find(params[:event_id]).download_location)",
"render_path": null,
"location": {
"type": "method",
"class": "RoomsController",
"method": "download"
},
"user_input": "UserEvent.find(params[:event_id]).download_location",
"confidence": "High",
"note": "Review comment in file for more details, deemed safe."
}
],
"updated": "2021-09-22 14:27:40 -0500",
"brakeman_version": "5.1.1"
}

Checks finished, collecting results...
Filtering warnings...
/opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/bundle/ruby/2.7.0/gems/highline-2.0.3/lib/highline/terminal.rb:136:in get_line_default': The input stream is exhausted. (EOFError) from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/bundle/ruby/2.7.0/gems/highline-2.0.3/lib/highline/terminal.rb:89:in get_line'
from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/bundle/ruby/2.7.0/gems/highline-2.0.3/lib/highline.rb:532:in get_line' from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/bundle/ruby/2.7.0/gems/highline-2.0.3/lib/highline.rb:516:in get_response_line_mode'
from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/bundle/ruby/2.7.0/gems/highline-2.0.3/lib/highline/question.rb:524:in get_response' from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/bundle/ruby/2.7.0/gems/highline-2.0.3/lib/highline/question.rb:536:in get_response_or_default'
from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/bundle/ruby/2.7.0/gems/highline-2.0.3/lib/highline/question_asker.rb:30:in ask_once' from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/bundle/ruby/2.7.0/gems/highline-2.0.3/lib/highline.rb:223:in ask'
from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/lib/brakeman/report/ignore/interactive.rb:38:in block in file_menu' from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/lib/brakeman/report/ignore/interactive.rb:37:in loop'
from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/lib/brakeman/report/ignore/interactive.rb:37:in file_menu' from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/lib/brakeman/report/ignore/interactive.rb:16:in start'
from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/lib/brakeman.rb:557:in filter_warnings' from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/lib/brakeman.rb:402:in scan'
from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/lib/brakeman.rb:86:in run' from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/lib/brakeman/commandline.rb:157:in run_brakeman'
from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/lib/brakeman/commandline.rb:125:in regular_report' from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/lib/brakeman/commandline.rb:166:in run_report'
from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/lib/brakeman/commandline.rb:35:in run' from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/lib/brakeman/commandline.rb:20:in start'
from /opt/hostedtoolcache/Ruby/3.0.2/x64/lib/ruby/gems/3.0.0/gems/brakeman-5.2.3/bin/brakeman:10:in <top (required)>' from /opt/hostedtoolcache/Ruby/3.0.2/x64/bin/brakeman:23:in load'
from /opt/hostedtoolcache/Ruby/3.0.2/x64/bin/brakeman:23:in `

'
Input file: |./config/brakeman.ignore|
Error: Process completed with exit code 1.

@ds-dustenharrison
Copy link
Author

ds-dustenharrison commented Jul 21, 2022

I changed Ruby to 3.0.4 - No change
I changed Ruby to 2.7.0 - No change
I changed Brakeman to 5.1.1 - No change
I have tried the -f flag and it didn't produce a file.
Any insight on why the failure occurred?

@presidentbeef
Copy link
Owner

Hi @ds-dustenharrison my suggestion is to run Brakeman locally and compare the fingerprints in the ignore file to the ones Brakeman outputs. If the fingerprints match or other reports filter the warnings, then it's an issue with the SARIF report. Otherwise, the fingerprints just need to be updated.

Instead of

brakeman -i ./config/brakeman.ignore --format sarif --output output.sarif.json .

you could run

brakeman -i ./config/brakeman.ignore --format json --output output.json .

and see if the results change.

@dentarg
Copy link

dentarg commented Nov 14, 2022

I'm not getting ignoring to work with GitHub. Maybe they doesn't support suppressions in the uploaded SARIF? I can see the warning being in ignored_warnings in the regular Brakeman JSON and in the suppressions array in the SARIF JSON.

Here's my config/brakeman.ignore: https://github.com/Starkast/wikimum/blob/1855075e2a30a7f82d8125545e4ec36eadd5983a/config/brakeman.ignore

The JSON from brakeman --force -f sarif -o output.sarif.json .
{
  "version": "2.1.0",
  "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json",
  "runs": [
    {
      "tool": {
        "driver": {
          "name": "Brakeman",
          "informationUri": "https://brakemanscanner.org",
          "semanticVersion": "5.3.1",
          "rules": [
            {
              "id": "BRAKE0016",
              "name": "SendFile/File Access",
              "fullDescription": {
                "text": "Check for user input in uses of send_file."
              },
              "helpUri": "https://brakemanscanner.org/docs/warning_types/file_access/",
              "help": {
                "text": "More info: https://brakemanscanner.org/docs/warning_types/file_access/.",
                "markdown": "[More info](https://brakemanscanner.org/docs/warning_types/file_access/)."
              },
              "properties": {
                "tags": [
                  "SendFile"
                ]
              }
            }
          ]
        }
      },
      "results": [
        {
          "ruleId": "BRAKE0016",
          "ruleIndex": 0,
          "level": "note",
          "message": {
            "text": "Parameter value used in file name."
          },
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "lib/controllers/backup_controller.rb",
                  "uriBaseId": "%SRCROOT%"
                },
                "region": {
                  "startLine": 37
                }
              }
            }
          ],
          "suppressions": [
            {
              "kind": "external",
              "justification": "The file access is limited to a temporary directory we create.",
              "location": {
                "physicalLocation": {
                  "artifactLocation": {
                    "uri": "config/brakeman.ignore",
                    "uriBaseId": "%SRCROOT%"
                  }
                }
              }
            }
          ]
        }
      ]
    }
  ]
}
The JSON from brakeman --force --format json .
{
  "scan_info": {
    "app_path": "/app",
    "rails_version": "4.x",
    "security_warnings": 0,
    "start_time": "2022-11-14 21:57:47 +0000",
    "end_time": "2022-11-14 21:57:47 +0000",
    "duration": 0.271699667,
    "checks_performed": [
      "BasicAuth",
      "BasicAuthTimingAttack",
      "CSRFTokenForgeryCVE",
      "ContentTag",
      "CookieSerialization",
      "CreateWith",
      "CrossSiteScripting",
      "DefaultRoutes",
      "Deserialize",
      "DetailedExceptions",
      "DigestDoS",
      "DynamicFinders",
      "EOLRails",
      "EOLRuby",
      "EscapeFunction",
      "Evaluation",
      "Execute",
      "FileAccess",
      "FileDisclosure",
      "FilterSkipping",
      "ForgerySetting",
      "HeaderDoS",
      "I18nXSS",
      "JRubyXML",
      "JSONEncoding",
      "JSONEntityEscape",
      "JSONParsing",
      "LinkTo",
      "LinkToHref",
      "MailTo",
      "MassAssignment",
      "MimeTypeDoS",
      "ModelAttrAccessible",
      "ModelAttributes",
      "ModelSerialize",
      "NestedAttributes",
      "NestedAttributesBypass",
      "NumberToCurrency",
      "PageCachingCVE",
      "PermitAttributes",
      "QuoteTableName",
      "Redirect",
      "RegexDoS",
      "Render",
      "RenderDoS",
      "RenderInline",
      "ResponseSplitting",
      "RouteDoS",
      "SQL",
      "SQLCVEs",
      "SSLVerify",
      "SafeBufferManipulation",
      "SanitizeConfigCve",
      "SanitizeMethods",
      "SelectTag",
      "SelectVulnerability",
      "Send",
      "SendFile",
      "SessionManipulation",
      "SessionSettings",
      "SimpleFormat",
      "SingleQuotes",
      "SkipBeforeFilter",
      "SprocketsPathTraversal",
      "StripTags",
      "SymbolDoSCVE",
      "TemplateInjection",
      "TranslateBug",
      "UnsafeReflection",
      "UnsafeReflectionMethods",
      "ValidationRegex",
      "VerbConfusion",
      "WithoutProtection",
      "XMLDoS",
      "YAMLParsing"
    ],
    "number_of_controllers": 5,
    "number_of_models": 0,
    "number_of_templates": 15,
    "ruby_version": "2.7.6",
    "brakeman_version": "5.3.1"
  },
  "warnings": [

  ],
  "ignored_warnings": [
    {
      "warning_type": "File Access",
      "warning_code": 16,
      "fingerprint": "b934e8f7994ab19feab782b372192a1ff20d6f70544c0ede1ad7b25ce24f0653",
      "check_name": "SendFile",
      "message": "Parameter value used in file name",
      "file": "lib/controllers/backup_controller.rb",
      "line": 37,
      "link": "https://brakemanscanner.org/docs/warning_types/file_access/",
      "code": "send_file(File.join(backup_tmpdir, Base64.urlsafe_decode64(params.fetch(\"encoded_path\"))), :type => \"text/plain\")",
      "render_path": null,
      "location": {
        "type": "method",
        "class": "BackupController",
        "method": null
      },
      "user_input": "params.fetch(\"encoded_path\")",
      "confidence": "Weak",
      "cwe_id": [
        22
      ]
    }
  ],
  "errors": [

  ],
  "obsolete": [

  ]
}

Using brakeman 5.3.1.

@dentarg
Copy link

dentarg commented Nov 14, 2022

Maybe they doesn't support suppressions in the uploaded SARIF?

That is the case: github/codeql-action#1230 (comment)

@presidentbeef
Copy link
Owner

Geez, even when you try to do everything right sometimes it's still not enough 😄

@presidentbeef
Copy link
Owner

Might have been fixed upstream. Not sure. But it's not a Brakeman issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants