Skip to content

Commit aa540fe

Browse files
committed
Excape match operations to prevent DoS queries
Previously when doing a `match` operation against an activerecord string field, we passed down the string directly and surrounded it with `%` on either side, allowing a substring match of the column. There was nothing preventing a client from including additional `%` wildcard characters in their query string to add more advanced substring matching. The problem with this is that if an attacker provides a few dozen wildcard characters, the sql engine can very quickly run into processing problems and cause a Denial of Service against the database/application. This commit takes advantage of activerecord and arel's sql escaping behavior to make this safe, at the tradeoff of not being able to pass additional wildcard characters as part of queries. Note that rails 4.x has fewer APIs for this behavior and they aren't all public. Because that is now an unsupported version of rails, this PR simply keeps the less safe older behavior when that version of activerecord is detected.
1 parent 1fbef6e commit aa540fe

File tree

2 files changed

+42
-5
lines changed

2 files changed

+42
-5
lines changed

lib/graphiti/adapters/active_record.rb

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,30 @@ def filter_string_not_suffix(scope, attribute, value)
7979
filter_string_suffix(scope, attribute, value, is_not: true)
8080
end
8181

82-
def filter_string_match(scope, attribute, value, is_not: false)
83-
column = column_for(scope, attribute)
84-
map = value.map { |v| "%#{v.downcase}%" }
85-
clause = column.lower.matches_any(map)
86-
is_not ? scope.where.not(clause) : scope.where(clause)
82+
# Arel has different match escaping behavior before rails 5.
83+
# Since rails 4.x does not expose methods to escape LIKE statements
84+
# anyway, we just don't support proper LIKE escaping in those versions.
85+
if ::ActiveRecord.version >= Gem::Version.new('5.0.0')
86+
def filter_string_match(scope, attribute, value, is_not: false)
87+
escape_char = '\\'
88+
column = column_for(scope, attribute)
89+
map = value.map do |v|
90+
v = v.downcase
91+
v = scope.sanitize_sql_like(v)
92+
"%#{v}%"
93+
end
94+
clause = column.lower.matches_any(map, escape_char, true)
95+
is_not ? scope.where.not(clause) : scope.where(clause)
96+
end
97+
else
98+
def filter_string_match(scope, attribute, value, is_not: false)
99+
column = column_for(scope, attribute)
100+
map = value.map do |v|
101+
"%#{v.downcase}%"
102+
end
103+
clause = column.lower.matches_any(map)
104+
is_not ? scope.where.not(clause) : scope.where(clause)
105+
end
87106
end
88107

89108
def filter_string_not_match(scope, attribute, value)

spec/integration/rails/finders_spec.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,24 @@ def resource
263263
it "executes case-insensitive match query" do
264264
expect(ids).to eq([author2.id, author3.id])
265265
end
266+
267+
if ::ActiveRecord.version >= Gem::Version.new("5.0")
268+
context 'when match string includes % characters' do
269+
let(:value) { {match: "ld%ca"} }
270+
271+
let!(:author_with_percent) do
272+
Legacy::Author.create!(first_name: "Wild%card")
273+
end
274+
275+
let!(:author_with_dash) do
276+
Legacy::Author.create!(first_name: "Wild-card")
277+
end
278+
279+
it "does not use the provided % as a wildcard character" do
280+
expect(ids).to eq([author_with_percent.id])
281+
end
282+
end
283+
end
266284
end
267285

268286
context "!match" do

0 commit comments

Comments
 (0)