Skip to content

Commit c8039ec

Browse files
stefansundinnbulaj
andcommitted
Merge pull request from GHSA-j7vx-8mqj-cqp9
* Attempt at fixing information disclosure vulnerability. * Add `#as_json` method and attrs serialization restriction for Application model * [ci skip] Add documentation for serialization Co-authored-by: Nikita Bulai <bulaj.nikita@gmail.com>
1 parent 1b1ace7 commit c8039ec

6 files changed

Lines changed: 458 additions & 283 deletions

File tree

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@ upgrade guides.
55

66
User-visible changes worth mentioning.
77

8+
## 5.2.5
9+
10+
- [#1371] Backport: add `#as_json` method and attributes serialization restriction for Application model.
11+
Fixes information disclosure vulnerability (CVE-2020-10187).
12+
13+
**[IMPORTANT]** you need to re-implement `#as_json` method for Doorkeeper Application model
14+
if you previously used `#to_json` serialization with custom options or attributes or rely on
15+
JSON response from /oauth/applications.json or /oauth/authorized_applications.json. This change
16+
is a breaking change which restricts serialized attributes to a very small set of columns.
17+
818
## 5.2.4
919

1020
- [#1360] Increase `matching_token_for` batch lookup size to 10 000 and make it configurable.

app/controllers/doorkeeper/applications_controller.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ def index
1919
def show
2020
respond_to do |format|
2121
format.html
22-
format.json { render json: @application }
22+
format.json { render json: @application, as_owner: true }
2323
end
2424
end
2525

@@ -36,7 +36,7 @@ def create
3636

3737
respond_to do |format|
3838
format.html { redirect_to oauth_application_url(@application) }
39-
format.json { render json: @application }
39+
format.json { render json: @application, as_owner: true }
4040
end
4141
else
4242
respond_to do |format|
@@ -58,7 +58,7 @@ def update
5858

5959
respond_to do |format|
6060
format.html { redirect_to oauth_application_url(@application) }
61-
format.json { render json: @application }
61+
format.json { render json: @application, as_owner: true }
6262
end
6363
else
6464
respond_to do |format|

app/controllers/doorkeeper/authorized_applications_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ def index
99

1010
respond_to do |format|
1111
format.html
12-
format.json { render json: @applications }
12+
format.json { render json: @applications, current_resource_owner: current_resource_owner }
1313
end
1414
end
1515

lib/doorkeeper/orm/active_record/application.rb

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,27 @@ def plaintext_secret
7070
end
7171
end
7272

73-
def to_json(options = nil)
74-
serializable_hash(except: :secret)
75-
.merge(secret: plaintext_secret)
76-
.to_json(options)
73+
# Represents client as set of it's attributes in JSON format.
74+
# This is the right way how we want to override ActiveRecord #to_json.
75+
#
76+
# Respects privacy settings and serializes minimum set of attributes
77+
# for public/private clients and full set for authorized owners.
78+
#
79+
# @return [Hash] entity attributes for JSON
80+
#
81+
def as_json(options = {})
82+
# if application belongs to some owner we need to check if it's the same as
83+
# the one passed in the options or check if we render the client as an owner
84+
if (respond_to?(:owner) && owner && owner == options[:current_resource_owner]) ||
85+
options[:as_owner]
86+
# Owners can see all the client attributes, fallback to ActiveModel serialization
87+
super
88+
else
89+
# if application has no owner or it's owner doesn't match one from the options
90+
# we render only minimum set of attributes that could be exposed to a public
91+
only = extract_serializable_attributes(options)
92+
super(options.merge(only: only))
93+
end
7794
end
7895

7996
private
@@ -98,5 +115,48 @@ def scopes_match_configured
98115
def enforce_scopes?
99116
Doorkeeper.configuration.enforce_configured_scopes?
100117
end
118+
119+
# Helper method to extract collection of serializable attribute names
120+
# considering serialization options (like `only`, `except` and so on).
121+
#
122+
# @param options [Hash] serialization options
123+
#
124+
# @return [Array<String>]
125+
# collection of attributes to be serialized using #as_json
126+
#
127+
def extract_serializable_attributes(options = {})
128+
opts = options.try(:dup) || {}
129+
only = Array.wrap(opts[:only]).map(&:to_s)
130+
131+
only = if only.blank?
132+
serializable_attributes
133+
else
134+
only & serializable_attributes
135+
end
136+
137+
only -= Array.wrap(opts[:except]).map(&:to_s) if opts.key?(:except)
138+
only.uniq
139+
end
140+
141+
# We need to hook into this method to allow serializing plan-text secrets
142+
# when secrets hashing enabled.
143+
#
144+
# @param key [String] attribute name
145+
#
146+
def read_attribute_for_serialization(key)
147+
return super unless key.to_s == "secret"
148+
149+
plaintext_secret || secret
150+
end
151+
152+
# Collection of attributes that could be serialized for public.
153+
# Override this method if you need additional attributes to be serialized.
154+
#
155+
# @return [Array<String>] collection of serializable attributes
156+
def serializable_attributes
157+
attributes = %w[id name created_at]
158+
attributes << "uid" unless confidential?
159+
attributes
160+
end
101161
end
102162
end

lib/doorkeeper/version.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ module VERSION
99
# Semantic versioning
1010
MAJOR = 5
1111
MINOR = 2
12-
TINY = 4
12+
TINY = 5
1313
PRE = nil
1414

1515
# Full version number

0 commit comments

Comments
 (0)