diff --git a/app/models/columns/champ_column.rb b/app/models/columns/champ_column.rb index 5419710fd..c032b3a35 100644 --- a/app/models/columns/champ_column.rb +++ b/app/models/columns/champ_column.rb @@ -37,7 +37,9 @@ class Columns::ChampColumn < Column if type == :enum relation.where(champs: { column => search_terms }).ids elsif type == :enums - relation.filter_array_enum(:champs, column, search_terms).ids + # in a multiple drop down list, the value are stored as '["v1", "v2"]' + quoted_search_terms = search_terms.map { %{"#{_1}"} } + relation.filter_ilike(:champs, column, quoted_search_terms).ids else relation.filter_ilike(:champs, column, search_terms).ids end diff --git a/app/models/concerns/dossier_filtering_concern.rb b/app/models/concerns/dossier_filtering_concern.rb index 4baab3a0f..7883ae700 100644 --- a/app/models/concerns/dossier_filtering_concern.rb +++ b/app/models/concerns/dossier_filtering_concern.rb @@ -35,12 +35,6 @@ module DossierFilteringConcern where("#{table_column} LIKE ANY (ARRAY[?])", safe_quoted_terms) } - scope :filter_array_enum, lambda { |table, column, values| - table_column = DossierFilterService.sanitized_column(table, column) - q = Array.new(values.count, "(#{table_column} = ?)").join(' OR ') - where(q, *(values. map { |value| "[\"#{value}\"]" })) - } - def sanitize_sql_like(q) = ActiveRecord::Base.sanitize_sql_like(q) end end diff --git a/config/brakeman.ignore b/config/brakeman.ignore index 38482ebdd..d064270c1 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -44,29 +44,6 @@ ], "note": "" }, - { - "warning_type": "SQL Injection", - "warning_code": 0, - "fingerprint": "5092b33433aef8fe42b688a780325f3791a77b39e55131256c78cebc3c14c0a3", - "check_name": "SQL", - "message": "Possible SQL injection", - "file": "app/models/concerns/dossier_filtering_concern.rb", - "line": 46, - "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", - "code": "where(\"#{values.count} OR #{\"(#{DossierFilterService.sanitized_column(table, column)} = ?)\"}\", *values.map do\n \"[\\\"#{value}\\\"]\"\n end)", - "render_path": null, - "location": { - "type": "method", - "class": "DossierFilteringConcern", - "method": null - }, - "user_input": "values.count", - "confidence": "Medium", - "cwe_id": [ - 89 - ], - "note": "filtered by rails query params where(something: ?, values)" - }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -120,7 +97,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/columns/json_path_column.rb", - "line": 24, + "line": 26, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "dossiers.with_type_de_champ(stable_id).where(\"champs.value_json @? '#{jsonpath} ? (@ like_regex \\\"#{quote_string(search_terms.join(\"|\"))}\\\" flag \\\"i\\\")'\")", "render_path": null, diff --git a/spec/services/dossier_filter_service_spec.rb b/spec/services/dossier_filter_service_spec.rb index d00ae6371..2290aad8c 100644 --- a/spec/services/dossier_filter_service_spec.rb +++ b/spec/services/dossier_filter_service_spec.rb @@ -510,20 +510,42 @@ describe DossierFilterService do end context 'with enums type_de_champ' do - let(:filter) { [type_de_champ.libelle, 'Favorable'] } - let(:types_de_champ_public) { [{ type: :multiple_drop_down_list, options: ['Favorable', 'Defavorable'] }] } + let(:filter) { [type_de_champ.libelle, search_term] } + let(:types_de_champ_public) { [{ type: :multiple_drop_down_list, options: ['champ', 'champignon'] }] } before do kept_champ = kept_dossier.champs.find_by(stable_id: type_de_champ.stable_id) - kept_champ.value = ['Favorable'] + kept_champ.value = ['champ', 'champignon'] kept_champ.save! discarded_champ = discarded_dossier.champs.find_by(stable_id: type_de_champ.stable_id) - discarded_champ.value = ['Defavorable'] + discarded_champ.value = ['champignon'] discarded_champ.save! end - it { is_expected.to contain_exactly(kept_dossier.id) } + context 'with single value' do + let(:search_term) { 'champ' } + + it { is_expected.to contain_exactly(kept_dossier.id) } + end + + context 'with multiple search values' do + let(:search_term) { 'champignon' } + + it { is_expected.to contain_exactly(kept_dossier.id, discarded_dossier.id) } + end + + context 'test if I could break a regex with %' do + let(:search_term) { '%' } + + it { is_expected.to be_empty } + end + + context 'test if I could break a regex with .' do + let(:search_term) { '.*' } + + it { is_expected.to be_empty } + end end end