Skip to content

Commit

Permalink
[Fix rubocop#3521] Disable autocorrect on Security/JSONLoad by defa…
Browse files Browse the repository at this point in the history
…ult.

The autocorrect on this cop is dangerous for at least two reasons depending on the value being passed to the `JSON` class method, so this disabled it by default. One known reason is where we pass in a stream such as `JSON.load(open('file'))`, this cannot be swapped out for `JSON.parse` without calling `#read` on the stream. Another known reason is where the JSON string is a single value, rather than a full JSON object, such as `JSON.load('false')`, this cannot be swapped out for `JSON.parse` without adding the `quirks_mode: true` option.

Also slightly improve the description in the cop and `enabled.yml`.

Also fix the offence message.
  • Loading branch information
savef committed Oct 7, 2016
1 parent d2ba8bf commit 35fdaec
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
* [#3510](https://github.com/bbatsov/rubocop/issues/3510): Fix some issues with `Style/SafeNavigation`. Fix auto-correct of multiline if expressions, and do not register an offense for scenarios using `||` and ternary expression. ([@rrosenblum][])
* [#3503](https://github.com/bbatsov/rubocop/issues/3503): Change misleading message of `Style/EmptyLinesAroundAccessModifier`. ([@bquorning][])
* [#3407](https://github.com/bbatsov/rubocop/issues/3407): Turn off autocorrect for unsafe rules by default. ([@ptarjan][])
* [#3521](https://github.com/bbatsov/rubocop/issues/3521): Turn off autocorrect for `Security/JSONLoad` by default. ([@savef][])

## 0.43.0 (2016-09-19)

Expand Down
8 changes: 7 additions & 1 deletion config/enabled.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1514,5 +1514,11 @@ Rails/Validation:
Enabled: true

Security/JSONLoad:
Description : 'Prefer usage of JSON.parse'
Description: >-
Prefer usage of `JSON.parse` over `JSON.load` due to potential
security issues. See reference for more information.
Reference: 'http://ruby-doc.org/stdlib-2.3.0/libdoc/json/rdoc/JSON.html#method-i-load'
Enabled: true
# Autocorrect here will change to a method that may cause crashes depending
# on the value of the argument.
AutoCorrect: false
13 changes: 11 additions & 2 deletions lib/rubocop/cop/security/json_load.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,16 @@
module RuboCop
module Cop
module Security
# This cop checks for the use of unsecure JSON methods.
# This cop checks for the use of JSON class methods which have potential
# security issues.
#
# Autocorrect is disabled by default because it's potentially dangerous.
# If using a stream, like `JSON.load(open('file'))`, it will need to call
# `#read` manually, like `JSON.parse(open('file').read)`.
# If reading single values (rather than proper JSON objects), like
# `JSON.load('false')`, it will need to pass the `quirks_mode: true`
# option, like `JSON.parse('false', quirks_mode: true)`.
# Other similar issues may apply.
#
# @example
# # always offense
Expand All @@ -14,7 +23,7 @@ module Security
# JSON.parse("{}")
#
class JSONLoad < Cop
MSG = 'Prefer `JSON.parse` instead of `JSON#%s`.'.freeze
MSG = 'Prefer `JSON.parse` over `JSON.%s`.'.freeze

def_node_matcher :json_load, <<-END
(send (const nil :JSON) ${:load :restore} ...)
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/security/json_load_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
it "registers an offense for JSON.#{method}" do
inspect_source(cop, "JSON.#{method}('{}')")
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.first.message).to include("JSON##{method}")
expect(cop.offenses.first.message).to include("JSON.#{method}")
end

it "autocorrects '.#{method}' to '.parse'" do
Expand Down

0 comments on commit 35fdaec

Please sign in to comment.