Add Security/Marshal cop #3816
Add Security/Marshal cop #3816
Conversation
| expect(cop.offenses).to be_empty | ||
| end | ||
|
|
||
| it 'accepts Marshal.dump' do |
There was a problem hiding this comment.
This description is exactly the same as the previous one.
| module Cop | ||
| module Security | ||
| # This cop checks for the use of Marshal class methods which have | ||
| # potential security issues. |
There was a problem hiding this comment.
I'd elaborate here on the security issues.
| # This cop checks for the use of Marshal class methods which have | ||
| # potential security issues. | ||
| # | ||
| # Autocorrect is disabled by default because it's potentially dangerous. |
There was a problem hiding this comment.
There's no auto-correct in this cop at all. :-)
| # Autocorrect is disabled by default because it's potentially dangerous. | ||
| # | ||
| # @example | ||
| # # always offense |
| # Marshal.load("{}") | ||
| # Marshal.restore("{}") | ||
| # | ||
| # # no offense |
|
The documentation diff is a bit strange. Is your branch up-to-date with |
|
@bbatsov I made the recommended changes. |
|
As a user seeing this cop fire on my code, the UX has some rough edges:
If the desired recommendation is "Do not use Marshal for untrusted deserialization", it seems that the cop has overstepped what can be tested for. At the very least, it ought to flag clearly that any use of Marshal is an RCE risk, rather than just saying "Avoid Marshal.load", so the user can understand why they would avoid Marshal. |
|
Good observations. I'd be best if you filed a new ticket regarding this. |
Add a new Security cop, that look for usage of
Marshal.loadandMarshal.restore, as they are both potentially dangerous: http://ruby-doc.org/core-2.3.3/Marshal.html#module-Marshal-label-Security+considerationsThere is no alternative for theses methods, as
Marshalis inherently dangerous.Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).and description in grammatically correct, complete sentences.
rake generate_cops_documentation(required only when you've added a new cop or changed the configuration/documentation of an existing cop).