Brakeman exit on warn cross-scripting error unsafe parameter value

617 views Asked by At

I'm using CircleCI to check for security issues and this is cropping up as an error, though I'm not sure that it is.

This is the line of code that is causing one of the scripting errors:

= link_to t(:delete), main_app.board_comment_path(@board, comment), method: :delete

Is this a valid security issue? Is there any way for me to make Brakeman accept these parameters as safe? I read up on --url-safe-methods but I couldn't figure out a way to make it work.

Used this link as a guide https://github.com/presidentbeef/brakeman/pull/45

Running bundle exec brakeman -A -q --exit-on-warn, this is the error report:

+BRAKEMAN REPORT+

Application path: ****
Rails version: 4.2.2
Brakeman version: 3.0.4
Started at 2015-06-26 14:10:14 -0700
Duration: 1.8311 seconds
Checks run: BasicAuth, ContentTag, CreateWith, CrossSiteScripting, DefaultRoutes, Deserialize, DetailedExceptions, DigestDoS, EscapeFunction, Evaluation, Execute, FileAccess, FileDisclosure, FilterSkipping, ForgerySetting, HeaderDoS, I18nXSS, JRubyXML, JSONEncoding, JSONParsing, LinkTo, LinkToHref, MailTo, MassAssignment, ModelAttrAccessible, ModelAttributes, ModelSerialize, NestedAttributes, NumberToCurrency, QuoteTableName, Redirect, RegexDoS, Render, RenderDoS, RenderInline, ResponseSplitting, SQL, SQLCVEs, SSLVerify, SafeBufferManipulation, SanitizeMethods, SelectTag, SelectVulnerability, Send, SendFile, SessionSettings, SimpleFormat, SingleQuotes, SkipBeforeFilter, StripTags, SymbolDoS, SymbolDoSCVE, TranslateBug, UnsafeReflection, UnscopedFind, ValidationRegex, WithoutProtection, XMLDoS, YAMLParsing


+SUMMARY+

+-------------------+-------+
| Scanned/Reported  | Total |
+-------------------+-------+
| Controllers       | 23    |
| Models            | 9     |
| Templates         | 53    |
| Errors            | 0     |
| Security Warnings | 2 (0) |
+-------------------+-------+

+----------------------+-------+
| Warning Type         | Total |
+----------------------+-------+
| Cross Site Scripting | 2     |
+----------------------+-------+


View Warnings:

+------------+------------------------------------------------------------------+----------------------+-------------------->>
| Confidence | Template                                                         | Warning Type         | Message            >>
+------------+------------------------------------------------------------------+----------------------+-------------------->>
| Medium     | boards/show (BoardsController#show) | Cross Site Scripting | Unsafe parameter va>>
| Medium     | boards/show (BoardsController#show) | Cross Site Scripting | Unsafe parameter va>>
+------------+------------------------------------------------------------------+----------------------+-------------------->>
1

There are 1 answers

0
Justin On BEST ANSWER

This is (almost certainly) a false positive, assuming board_comment_path returns a path.

The reason Brakeman warns about URLs in link_to is because it is possible to set URLs like javascript:dangerous_stuff_here(). A common example would be user profiles linking to a user's website.

--url-safe-methods only applies to methods wrapping input to link_to. For example, link_to 'stuff', safe_url(some_input).

However, after https://github.com/presidentbeef/brakeman/pull/674 Brakeman will stop warning about path helpers in URLs and also expand --safe-methods/--url-safe-methods to match all types of methods.