From fa8ed7f0b13c9b200daa7d62883237c78d4015d4 Mon Sep 17 00:00:00 2001 From: Jens Kraemer Date: Mon, 12 Apr 2021 12:10:02 +0800 Subject: [PATCH 3/5] use sanitize_sql_like in like scopes - Issue, Principal, Project and Version --- app/models/issue.rb | 2 +- app/models/principal.rb | 4 ++-- app/models/project.rb | 2 +- app/models/version.rb | 2 +- test/unit/issue_test.rb | 16 ++++++++++++++++ test/unit/principal_test.rb | 16 ++++++++++++++++ test/unit/project_test.rb | 16 ++++++++++++++++ test/unit/version_test.rb | 16 ++++++++++++++++ 8 files changed, 69 insertions(+), 5 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index 09f8400cc..3b08fc43c 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -102,7 +102,7 @@ class Issue < ActiveRecord::Base scope :like, (lambda do |q| q = q.to_s if q.present? - where("LOWER(#{table_name}.subject) LIKE LOWER(?)", "%#{q}%") + where("LOWER(#{table_name}.subject) LIKE LOWER(?)", "%#{sanitize_sql_like q}%") end end) diff --git a/app/models/principal.rb b/app/models/principal.rb index 009d2ab83..82c17b472 100644 --- a/app/models/principal.rb +++ b/app/models/principal.rb @@ -71,12 +71,12 @@ class Principal < ActiveRecord::Base if q.blank? where({}) else - pattern = "%#{q}%" + pattern = "%#{sanitize_sql_like q}%" sql = +"LOWER(#{table_name}.login) LIKE LOWER(:p)" sql << " OR #{table_name}.id IN (SELECT user_id FROM #{EmailAddress.table_name} WHERE LOWER(address) LIKE LOWER(:p))" params = {:p => pattern} - tokens = q.split(/\s+/).reject(&:blank?).map {|token| "%#{token}%"} + tokens = q.split(/\s+/).reject(&:blank?).map {|token| "%#{sanitize_sql_like token}%"} if tokens.present? sql << ' OR (' sql << tokens.map.with_index do |token, index| diff --git a/app/models/project.rb b/app/models/project.rb index ab864436a..da03be330 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -105,7 +105,7 @@ class Project < ActiveRecord::Base end) scope :like, (lambda do |arg| if arg.present? - pattern = "%#{arg.to_s.strip}%" + pattern = "%#{sanitize_sql_like arg.to_s.strip}%" where("LOWER(identifier) LIKE LOWER(:p) OR LOWER(name) LIKE LOWER(:p)", :p => pattern) end end) diff --git a/app/models/version.rb b/app/models/version.rb index 2e5430746..39dff7a0f 100644 --- a/app/models/version.rb +++ b/app/models/version.rb @@ -137,7 +137,7 @@ class Version < ActiveRecord::Base scope :named, lambda {|arg| where("LOWER(#{table_name}.name) = LOWER(?)", arg.to_s.strip)} scope :like, (lambda do |arg| if arg.present? - pattern = "%#{arg.to_s.strip}%" + pattern = "%#{sanitize_sql_like arg.to_s.strip}%" where([Redmine::Database.like("#{Version.table_name}.name", '?'), pattern]) end end) diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 797e4eca7..dd2e286ff 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -3406,4 +3406,20 @@ class IssueTest < ActiveSupport::TestCase assert_equal [5], issue2.filter_projects_scope('').ids.sort end + + def test_like_should_escape_query + issue = Issue.generate!(:subject => "asdf") + r = Issue.like('as_f') + assert_not_include issue, r + r = Issue.like('as%f') + assert_not_include issue, r + + issue = Issue.generate!(:subject => "as%f") + r = Issue.like('as%f') + assert_include issue, r + + issue = Issue.generate!(:subject => "as_f") + r = Issue.like('as_f') + assert_include issue, r + end end diff --git a/test/unit/principal_test.rb b/test/unit/principal_test.rb index f9953620d..ee6d3179c 100644 --- a/test/unit/principal_test.rb +++ b/test/unit/principal_test.rb @@ -147,4 +147,20 @@ class PrincipalTest < ActiveSupport::TestCase assert_equal 1, results.count assert_equal user, results.first end + + def test_like_scope_should_escape_query + user = User.generate!(:firstname => 'Leonardo', :lastname => 'da Vinci') + r = Principal.like('Vi_ci') + assert_not_include user, r + r = Principal.like('Vi%ci') + assert_not_include user, r + + user.update_column :lastname, 'da Vi%ci' + r = Principal.like('vi%ci') + assert_include user, r + + user.update_column :lastname, 'da Vi_ci' + r = Principal.like('vi_ci') + assert_include user, r + end end diff --git a/test/unit/project_test.rb b/test/unit/project_test.rb index b154e6b90..904dcd914 100644 --- a/test/unit/project_test.rb +++ b/test/unit/project_test.rb @@ -1120,4 +1120,20 @@ class ProjectTest < ActiveSupport::TestCase assert_equal 'valuea', project.custom_field_value(cf1) assert_nil project.custom_field_value(cf2) end + + def test_like_scope_should_escape_query + project = Project.find 'ecookbook' + r = Project.like('eco_k') + assert_not_include project, r + r = Project.like('eco%k') + assert_not_include project, r + + project.update_column :name, 'Eco%kbook' + r = Project.like('eco%k') + assert_include project, r + + project.update_column :name, 'Eco_kbook' + r = Project.like('eco_k') + assert_include project, r + end end diff --git a/test/unit/version_test.rb b/test/unit/version_test.rb index 057a97e6e..061c259d5 100644 --- a/test/unit/version_test.rb +++ b/test/unit/version_test.rb @@ -300,6 +300,22 @@ class VersionTest < ActiveSupport::TestCase assert_includes Version.like('like scope'), version end + def test_like_scope_should_escape_query + version = Version.create!(:project => Project.find(1), :name => 'Version for like scope test') + r = Version.like('Ver_ion') + assert_not_include version, r + r = Version.like('Ver%ion') + assert_not_include version, r + + version.update_column :name, 'Ver%ion' + r = Version.like('ver%i') + assert_include version, r + + version.update_column :name, 'Ver_ion' + r = Version.like('ver_i') + assert_include version, r + end + def test_safe_attributes_should_include_only_custom_fields_visible_to_user cf1 = VersionCustomField.create!(:name => 'Visible field', :field_format => 'string', -- 2.20.1