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

Add JUnit-XML format report #1453

Merged

Conversation

@naokikimura
Copy link
Contributor

naokikimura commented Feb 5, 2020

I want to gain insight into Brakeman's results at CircleCI. Therefore, JUnit-XML format report is required.

Reference: Collecting Test Metadata - CircleCI

The result of ./bin/brakeman -f junit ./test/apps/rails6 is:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites xmlns:brakeman="https://brakemanscanner.org/">
  <brakeman:properties xml:id="scan_info">
    <brakeman:property brakeman:name="app_path" brakeman:value="/Users/kimuranaoki/Documents/workspace/brakeman/test/apps/rails6"/>
    <brakeman:property brakeman:name="rails_version" brakeman:value="6.0.0.beta2"/>
    <brakeman:property brakeman:name="security_warnings" brakeman:value="13"/>
    <brakeman:property brakeman:name="start_time" brakeman:value="2020-02-06T12:36:11+09:00"/>
    <brakeman:property brakeman:name="end_time" brakeman:value="2020-02-06T12:36:12+09:00"/>
    <brakeman:property brakeman:name="duration" brakeman:value="0.151494"/>
    <brakeman:property brakeman:name="checks_performed" brakeman:value="BasicAuth,BasicAuthTimingAttack,CrossSiteScripting,ContentTag,CookieSerialization,CreateWith,DefaultRoutes,Deserialize,DetailedExceptions,DigestDoS,DynamicFinders,EscapeFunction,Evaluation,Execute,FileAccess,FileDisclosure,FilterSkipping,ForgerySetting,HeaderDoS,I18nXSS,JRubyXML,JSONEncoding,JSONParsing,LinkTo,LinkToHref,MailTo,MassAssignment,MimeTypeDoS,ModelAttrAccessible,ModelAttributes,ModelSerialize,NestedAttributes,NestedAttributesBypass,NumberToCurrency,PermitAttributes,QuoteTableName,Redirect,RegexDoS,Render,RenderDoS,RenderInline,ResponseSplitting,RouteDoS,SafeBufferManipulation,SanitizeMethods,SelectTag,SelectVulnerability,Send,SendFile,SessionManipulation,SessionSettings,SimpleFormat,SingleQuotes,SkipBeforeFilter,SprocketsPathTraversal,SQL,SQLCVEs,SSLVerify,StripTags,SymbolDoSCVE,TranslateBug,UnsafeReflection,ValidationRegex,WithoutProtection,XMLDoS,YAMLParsing"/>
    <brakeman:property brakeman:name="number_of_controllers" brakeman:value="3"/>
    <brakeman:property brakeman:name="number_of_models" brakeman:value="3"/>
    <brakeman:property brakeman:name="ruby_version" brakeman:value="7"/>
    <brakeman:property brakeman:name="number_of_templates" brakeman:value="2.3.8"/>
    <brakeman:property brakeman:name="brakeman_version" brakeman:value="4.7.2"/>
  </brakeman:properties>
  <brakeman:errors/>
  <brakeman:obsolete/>
  <brakeman:ignored/>
  <testsuite errors="0" failures="1" hostname="C02YT0DXLVDM.local" id="1" name="config/initializers/cookies_serializer.rb" package="brakeman" tests="66" time="0" timestamp="2020-02-06T12:36:11">
    <properties/>
    <testcase classname="Brakeman::CheckCookieSerialization" name="run_check" time="0">
      <failure brakeman:code="Rails.application.config.action_dispatch.cookies_serializer = :marshal" brakeman:confidence="Medium" brakeman:file="config/initializers/cookies_serializer.rb" brakeman:fingerprint="d882f63ce96c28fb6c6e0982f2a171460e4b933bfd9b9a5421dca21eef3f76da" brakeman:line="5" message="Use of unsafe cookie serialization strategy `:marshal` might lead to remote code execution" type="Remote Code Execution">(Medium) Remote Code Execution - Use of unsafe cookie serialization strategy `:marshal` might lead to remote code execution near line 5 in config/initializers/cookies_serializer.rb: Rails.application.config.action_dispatch.cookies_serializer = :marshal</failure>
    </testcase>
    <system-out/>
    <system-err/>
  </testsuite>
  <testsuite errors="0" failures="7" hostname="C02YT0DXLVDM.local" id="2" name="app/controllers/users_controller.rb" package="brakeman" tests="66" time="0" timestamp="2020-02-06T12:36:11">
    <properties/>
    <testcase classname="Brakeman::CheckExecute" name="run_check" time="0">
      <failure brakeman:code="system(&quot;bash&quot;, &quot;-c&quot;, params[:script])" brakeman:confidence="High" brakeman:file="app/controllers/users_controller.rb" brakeman:fingerprint="22f0226c43eeb59bff49e4f0ea10014c2882c8be2f51e4d36876a26960b100d9" brakeman:line="70" message="Possible command injection" type="Command Injection">(High) Command Injection - Possible command injection near line 70 in app/controllers/users_controller.rb: system("bash", "-c", params[:script])</failure>
    </testcase>
    <testcase classname="Brakeman::CheckExecute" name="run_check" time="0">
      <failure brakeman:code="exec(&quot;zsh&quot;, &quot;-c&quot;, &quot;#{params[:script]} -e ./&quot;)" brakeman:confidence="High" brakeman:file="app/controllers/users_controller.rb" brakeman:fingerprint="d5b5eeed916c878c897bcde8b922bb18cdcf9fc1acfb8e37b30eb02422e8c43e" brakeman:line="75" message="Possible command injection" type="Command Injection">(High) Command Injection - Possible command injection near line 75 in app/controllers/users_controller.rb: exec("zsh", "-c", "#{params[:script]} -e ./")</failure>
    </testcase>
    <testcase classname="Brakeman::CheckMassAssignment" name="run_check" time="0">
      <failure brakeman:code="params.permit!" brakeman:confidence="Medium" brakeman:file="app/controllers/users_controller.rb" brakeman:fingerprint="58e42d4ef79c278374a8456b1c034c7768e28b9a156e5602bb99a1105349f350" brakeman:line="93" message="Parameters should be whitelisted for mass assignment" type="Mass Assignment">(Medium) Mass Assignment - Parameters should be whitelisted for mass assignment near line 93 in app/controllers/users_controller.rb: params.permit!</failure>
    </testcase>
    <testcase classname="Brakeman::CheckMassAssignment" name="run_check" time="0">
      <failure brakeman:code="params.permit!" brakeman:confidence="Medium" brakeman:file="app/controllers/users_controller.rb" brakeman:fingerprint="58e42d4ef79c278374a8456b1c034c7768e28b9a156e5602bb99a1105349f350" brakeman:line="94" message="Parameters should be whitelisted for mass assignment" type="Mass Assignment">(Medium) Mass Assignment - Parameters should be whitelisted for mass assignment near line 94 in app/controllers/users_controller.rb: params.permit!</failure>
    </testcase>
    <testcase classname="Brakeman::CheckRedirect" name="run_check" time="0">
      <failure brakeman:code="redirect_to(request.params)" brakeman:confidence="High" brakeman:file="app/controllers/users_controller.rb" brakeman:fingerprint="1d18e872e5f74ff0fd445008fd00ea2f04d5b3086f18682e301621779cd609a2" brakeman:line="88" message="Possible unprotected redirect" type="Redirect">(High) Redirect - Possible unprotected redirect near line 88 in app/controllers/users_controller.rb: redirect_to(request.params)</failure>
    </testcase>
    <testcase classname="Brakeman::CheckSQL" name="run_check" time="0">
      <failure brakeman:code="@user.delete_by(params[:user])" brakeman:confidence="Medium" brakeman:file="app/controllers/users_controller.rb" brakeman:fingerprint="02ad62a4e0cc17d972701be99e1d1ba4761b9176acc36e41498eac3a8d853a8a" brakeman:line="66" message="Possible SQL injection" type="SQL Injection">(Medium) SQL Injection - Possible SQL injection near line 66 in app/controllers/users_controller.rb: @user.delete_by(params[:user])</failure>
    </testcase>
    <testcase classname="Brakeman::CheckSQL" name="run_check" time="0">
      <failure brakeman:code="@user.destroy_by(params[:user])" brakeman:confidence="Medium" brakeman:file="app/controllers/users_controller.rb" brakeman:fingerprint="5049d89b5d867ce8c2e602746575b512f147b0ff4eca18ac1b2a3a308204180e" brakeman:line="65" message="Possible SQL injection" type="SQL Injection">(Medium) SQL Injection - Possible SQL injection near line 65 in app/controllers/users_controller.rb: @user.destroy_by(params[:user])</failure>
    </testcase>
    <system-out/>
    <system-err/>
  </testsuite>
  <testsuite errors="0" failures="1" hostname="C02YT0DXLVDM.local" id="3" name="app/models/user.rb" package="brakeman" tests="66" time="0" timestamp="2020-02-06T12:36:11">
    <properties/>
    <testcase classname="Brakeman::CheckSQL" name="run_check" time="0">
      <failure brakeman:code="where(&quot;      name = '#{name}'\n&quot;.strip_heredoc)" brakeman:confidence="Medium" brakeman:file="app/models/user.rb" brakeman:fingerprint="c567289064ac39d277b33a5b860641b79a8139cf85a9a079bc7bb36130784a93" brakeman:line="11" message="Possible SQL injection" type="SQL Injection">(Medium) SQL Injection - Possible SQL injection near line 11 in app/models/user.rb: where("      name = '#{name}'\n".strip_heredoc)</failure>
    </testcase>
    <system-out/>
    <system-err/>
  </testsuite>
  <testsuite errors="0" failures="4" hostname="C02YT0DXLVDM.local" id="4" name="app/views/users/show.html.erb" package="brakeman" tests="66" time="0" timestamp="2020-02-06T12:36:11">
    <properties/>
    <testcase classname="Brakeman::CheckCrossSiteScripting" name="run_check" time="0">
      <failure brakeman:code="User.new(user_params).name" brakeman:confidence="High" brakeman:file="app/views/users/show.html.erb" brakeman:fingerprint="9e949d88329883f879b7ff46bdb096ba43e791aacb6558f47beddc34b9d42c4c" brakeman:line="5" message="Unescaped model attribute" type="Cross-Site Scripting">(High) Cross-Site Scripting - Unescaped model attribute near line 5 in app/views/users/show.html.erb: User.new(user_params).name</failure>
    </testcase>
    <testcase classname="Brakeman::CheckCrossSiteScripting" name="run_check" time="0">
      <failure brakeman:code="User.new(user_params).name" brakeman:confidence="High" brakeman:file="app/views/users/show.html.erb" brakeman:fingerprint="9e949d88329883f879b7ff46bdb096ba43e791aacb6558f47beddc34b9d42c4c" brakeman:line="6" message="Unescaped model attribute" type="Cross-Site Scripting">(High) Cross-Site Scripting - Unescaped model attribute near line 6 in app/views/users/show.html.erb: User.new(user_params).name</failure>
    </testcase>
    <testcase classname="Brakeman::CheckCrossSiteScripting" name="run_check" time="0">
      <failure brakeman:code="User.new(user_params).name" brakeman:confidence="High" brakeman:file="app/views/users/show.html.erb" brakeman:fingerprint="9e949d88329883f879b7ff46bdb096ba43e791aacb6558f47beddc34b9d42c4c" brakeman:line="7" message="Unescaped model attribute" type="Cross-Site Scripting">(High) Cross-Site Scripting - Unescaped model attribute near line 7 in app/views/users/show.html.erb: User.new(user_params).name</failure>
    </testcase>
    <testcase classname="Brakeman::CheckCrossSiteScripting" name="run_check" time="0">
      <failure brakeman:code="User.new(user_params).name" brakeman:confidence="High" brakeman:file="app/views/users/show.html.erb" brakeman:fingerprint="9e949d88329883f879b7ff46bdb096ba43e791aacb6558f47beddc34b9d42c4c" brakeman:line="8" message="Unescaped model attribute" type="Cross-Site Scripting">(High) Cross-Site Scripting - Unescaped model attribute near line 8 in app/views/users/show.html.erb: User.new(user_params).name</failure>
    </testcase>
    <system-out/>
    <system-err/>
  </testsuite>
</testsuites>
If you remove the elements and attributes that belong to the brakeman namespace, you will get a valid XML in JUnit.xsd as follows:
<?xml version="1.0" encoding="UTF-8"?>
<testsuites xmlns:brakeman="https://brakemanscanner.org/">
  <testsuite errors="0" failures="1" hostname="C02YT0DXLVDM.local" id="1" name="config/initializers/cookies_serializer.rb" package="brakeman" tests="66" time="0" timestamp="2020-02-06T12:38:24">
    <properties/>
    <testcase classname="Brakeman::CheckCookieSerialization" name="run_check" time="0">
      <failure message="Use of unsafe cookie serialization strategy `:marshal` might lead to remote code execution" type="Remote Code Execution">(Medium) Remote Code Execution - Use of unsafe cookie serialization strategy `:marshal` might lead to remote code execution near line 5 in config/initializers/cookies_serializer.rb: Rails.application.config.action_dispatch.cookies_serializer = :marshal</failure>
    </testcase>
    <system-out/>
    <system-err/>
  </testsuite>
  <testsuite errors="0" failures="7" hostname="C02YT0DXLVDM.local" id="2" name="app/controllers/users_controller.rb" package="brakeman" tests="66" time="0" timestamp="2020-02-06T12:38:24">
    <properties/>
    <testcase classname="Brakeman::CheckExecute" name="run_check" time="0">
      <failure message="Possible command injection" type="Command Injection">(High) Command Injection - Possible command injection near line 70 in app/controllers/users_controller.rb: system("bash", "-c", params[:script])</failure>
    </testcase>
    <testcase classname="Brakeman::CheckExecute" name="run_check" time="0">
      <failure message="Possible command injection" type="Command Injection">(High) Command Injection - Possible command injection near line 75 in app/controllers/users_controller.rb: exec("zsh", "-c", "#{params[:script]} -e ./")</failure>
    </testcase>
    <testcase classname="Brakeman::CheckMassAssignment" name="run_check" time="0">
      <failure message="Parameters should be whitelisted for mass assignment" type="Mass Assignment">(Medium) Mass Assignment - Parameters should be whitelisted for mass assignment near line 93 in app/controllers/users_controller.rb: params.permit!</failure>
    </testcase>
    <testcase classname="Brakeman::CheckMassAssignment" name="run_check" time="0">
      <failure message="Parameters should be whitelisted for mass assignment" type="Mass Assignment">(Medium) Mass Assignment - Parameters should be whitelisted for mass assignment near line 94 in app/controllers/users_controller.rb: params.permit!</failure>
    </testcase>
    <testcase classname="Brakeman::CheckRedirect" name="run_check" time="0">
      <failure message="Possible unprotected redirect" type="Redirect">(High) Redirect - Possible unprotected redirect near line 88 in app/controllers/users_controller.rb: redirect_to(request.params)</failure>
    </testcase>
    <testcase classname="Brakeman::CheckSQL" name="run_check" time="0">
      <failure message="Possible SQL injection" type="SQL Injection">(Medium) SQL Injection - Possible SQL injection near line 66 in app/controllers/users_controller.rb: @user.delete_by(params[:user])</failure>
    </testcase>
    <testcase classname="Brakeman::CheckSQL" name="run_check" time="0">
      <failure message="Possible SQL injection" type="SQL Injection">(Medium) SQL Injection - Possible SQL injection near line 65 in app/controllers/users_controller.rb: @user.destroy_by(params[:user])</failure>
    </testcase>
    <system-out/>
    <system-err/>
  </testsuite>
  <testsuite errors="0" failures="1" hostname="C02YT0DXLVDM.local" id="3" name="app/models/user.rb" package="brakeman" tests="66" time="0" timestamp="2020-02-06T12:38:24">
    <properties/>
    <testcase classname="Brakeman::CheckSQL" name="run_check" time="0">
      <failure message="Possible SQL injection" type="SQL Injection">(Medium) SQL Injection - Possible SQL injection near line 11 in app/models/user.rb: where("      name = '#{name}'\n".strip_heredoc)</failure>
    </testcase>
    <system-out/>
    <system-err/>
  </testsuite>
  <testsuite errors="0" failures="4" hostname="C02YT0DXLVDM.local" id="4" name="app/views/users/show.html.erb" package="brakeman" tests="66" time="0" timestamp="2020-02-06T12:38:24">
    <properties/>
    <testcase classname="Brakeman::CheckCrossSiteScripting" name="run_check" time="0">
      <failure message="Unescaped model attribute" type="Cross-Site Scripting">(High) Cross-Site Scripting - Unescaped model attribute near line 5 in app/views/users/show.html.erb: User.new(user_params).name</failure>
    </testcase>
    <testcase classname="Brakeman::CheckCrossSiteScripting" name="run_check" time="0">
      <failure message="Unescaped model attribute" type="Cross-Site Scripting">(High) Cross-Site Scripting - Unescaped model attribute near line 6 in app/views/users/show.html.erb: User.new(user_params).name</failure>
    </testcase>
    <testcase classname="Brakeman::CheckCrossSiteScripting" name="run_check" time="0">
      <failure message="Unescaped model attribute" type="Cross-Site Scripting">(High) Cross-Site Scripting - Unescaped model attribute near line 7 in app/views/users/show.html.erb: User.new(user_params).name</failure>
    </testcase>
    <testcase classname="Brakeman::CheckCrossSiteScripting" name="run_check" time="0">
      <failure message="Unescaped model attribute" type="Cross-Site Scripting">(High) Cross-Site Scripting - Unescaped model attribute near line 8 in app/views/users/show.html.erb: User.new(user_params).name</failure>
    </testcase>
    <system-out/>
    <system-err/>
  </testsuite>
</testsuites>
@naokikimura naokikimura requested a review from presidentbeef Feb 5, 2020
@naokikimura naokikimura force-pushed the naokikimura:feature/junit-xml-format-report branch from 2bbded8 to 00f3daa Feb 5, 2020
Copy link
Owner

presidentbeef left a comment

@naokikimura Awesome, thank you! 😎

This format should also be added to Brakeman.get_formats_from_output_format so one can specify -o report.junit. I recognize there are way too much code to touch for adding a new report format, my apologies.

Have you been able to validate the output? I tried your report against a random online validator against this schema and got the following issues:

  • Cvc-complex-type.2.4.a: Invalid Content Was Found Starting With Element 'brakeman:properties'. One Of '{testsuite}' Is Expected., Line '2', Column '43'.
  • Cvc-pattern-valid: Value '2020-02-05T20:34:51+09:00' Is Not Facet-valid With Respect To Pattern '[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}' For Type 'ISO8601_DATETIME_PATTERN'., Line '19', Column '201'.
  • Cvc-attribute.3: The Value '2020-02-05T20:34:51+09:00' Of Attribute 'timestamp' On Element 'testsuite' Is Not Valid With Respect To Its Type, 'ISO8601_DATETIME_PATTERN'., Line '19', Column '201'.
  • Cvc-complex-type.2.4.a: Invalid Content Was Found Starting With Element 'testcase'. One Of '{properties}' Is Expected., Line '20', Column '88'.
lib/brakeman/report/report_junit.rb Outdated Show resolved Hide resolved
lib/brakeman/report/report_junit.rb Outdated Show resolved Hide resolved
lib/brakeman/report/report_junit.rb Outdated Show resolved Hide resolved
lib/brakeman/report/report_junit.rb Outdated Show resolved Hide resolved
lib/brakeman/report/report_junit.rb Show resolved Hide resolved
@naokikimura

This comment has been minimized.

Copy link
Contributor Author

naokikimura commented Feb 6, 2020

@presidentbeef Thank you for reviewing.

There are several variations of the JUnit-XML schema, and other reporters(eg ESLint) are often not compliant.

JUnit-XML is less expressive, so I added Brakeman-specific information using namespaces. In most cases, elements and attributes added using namespaces are ignored and have no effect on existing systems that handle JUnit-XML.

If you want to get XML compliant with JUnit.xsd, remove the elements and attributes belonging to the breakman namespace.

It's easy with XMLStarlet.

brakeman -f junit | xmlstarlet ed -d '//brakeman:*' -d '//@brakeman:*' > report-junit.xml
@naokikimura naokikimura requested a review from presidentbeef Feb 6, 2020
@presidentbeef

This comment has been minimized.

Copy link
Owner

presidentbeef commented Feb 13, 2020

@naokikimura Do you happen to have a public CircleCI build where I can see the results? Have you tested it yourself on CircleCI?

Just want some assurance before I merge 😀

@naokikimura

This comment has been minimized.

Copy link
Contributor Author

naokikimura commented Feb 14, 2020

@naokikimura Do you happen to have a public CircleCI build where I can see the results? Have you tested it yourself on CircleCI?

@presidentbeef Here is the result of scanning ./test/apps/rails4 with CircleCI:

https://app.circleci.com/jobs/github/naokikimura/brakeman/12/tests

Screenshot

app circleci com_jobs_github_naokikimura_brakeman_12_tests(iPad Pro)

@naokikimura naokikimura requested a review from presidentbeef Feb 14, 2020
@presidentbeef presidentbeef merged commit a097acd into presidentbeef:master Feb 14, 2020
9 checks passed
9 checks passed
ci/circleci: default Your tests passed on CircleCI!
Details
ci/circleci: test-2-3 Your tests passed on CircleCI!
Details
ci/circleci: test-2-4 Your tests passed on CircleCI!
Details
ci/circleci: test-2-5 Your tests passed on CircleCI!
Details
ci/circleci: test-2-6 Your tests passed on CircleCI!
Details
ci/circleci: upload-coverage Your tests passed on CircleCI!
Details
codeclimate All good!
Details
codeclimate/diff-coverage 98% (98% threshold)
Details
codeclimate/total-coverage 95% (0.0% change)
Details
@presidentbeef

This comment has been minimized.

Copy link
Owner

presidentbeef commented Feb 14, 2020

Thank you so much for your work on adding this report format! 🙇

@naokikimura naokikimura deleted the naokikimura:feature/junit-xml-format-report branch Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.