diff --git a/app/controllers/smart_search_controller.rb b/app/controllers/smart_search_controller.rb index ac28293a395..d81255b2758 100644 --- a/app/controllers/smart_search_controller.rb +++ b/app/controllers/smart_search_controller.rb @@ -46,7 +46,7 @@ class SmartSearchController < ApplicationController # TODO: Prevent duplicates after chunking embeddings is implemented # TODO: Paginate and remove the hardcoded limit (ADV-23) sql = <<-SQL.squish - SELECT wp.*, (wpe.embedding <=> ?) AS distance + SELECT wp.*, (wpe.embedding #{quoted_operator_name("<=>")} ?) AS distance FROM #{WikiPage.quoted_table_name} wp INNER JOIN #{Enrollment.quoted_table_name} AS e ON wp.context_type = 'Course' @@ -76,4 +76,10 @@ class SmartSearchController < ApplicationController render_unauthorized_action unless OpenAi.smart_search_available?(@domain_root_account) # TODO: Add state required for new page render end + + protected + + def quoted_operator_name(operator) + "operator(#{PG::Connection.quote_ident(ActiveRecord::Base.connection.extension("vector").schema)}.#{operator})" + end end diff --git a/config/brakeman.ignore b/config/brakeman.ignore index b9795fc1a6f..5c48881a0b4 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -53,7 +53,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/account.rb", - "line": 1047, + "line": 1062, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "Account.connection.select_values(\"WITH RECURSIVE t AS (\\n SELECT * FROM #{Account.quoted_table_name} WHERE id=#{Shard.local_id_for(starting_account_id.parent_account_id).first}\\n UNION\\n SELECT accounts.* FROM #{Account.quoted_table_name} INNER JOIN t ON accounts.id=t.parent_account_id\\n)\\nSELECT id FROM t\\n\".squish)", "render_path": null, @@ -76,7 +76,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/quizzes/quiz.rb", - "line": 1392, + "line": 1393, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "Quizzes::QuizSubmission.from(\"( VALUES #{quizzes.map do\n \"(#{q.id})\"\n end.join(\", \")} ) AS s(quiz_id)\")", "render_path": null, @@ -99,7 +99,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "config/initializers/active_record.rb", - "line": 1026, + "line": 1028, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "connection.execute(\"CREATE INDEX \\\"temp_primary_key\\\" ON #{connection.quote_local_table_name(\"#{table_name}_in_batches_temp_table_#{apply_limits(self, start, finish, order).to_sql.hash.abs.to_s(36)}\"[(-63..)])}(#{connection.quote_column_name(primary_key)})\")", "render_path": null, @@ -256,7 +256,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/controllers/calendar_events_api_controller.rb", - "line": 1251, + "line": 1344, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "InstStatsd::Statsd.count(\"account_calendars.modal.enabled_calendars\", params[:enabled_account_calendars].length)", "render_path": null, @@ -509,7 +509,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "config/initializers/active_record.rb", - "line": 858, + "line": 860, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "connection.execute(\"DECLARE #{\"#{table_name}_in_batches_cursor_#{apply_limits(clone, start, finish, order).except(:select).select(primary_key).to_sql.hash.abs.to_s(36)}\"} CURSOR FOR #{apply_limits(clone, start, finish, order).except(:select).select(primary_key).to_sql}\")", "render_path": null, @@ -551,7 +551,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "config/initializers/active_record.rb", - "line": 1060, + "line": 1062, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "connection.execute(\"DROP TABLE #{\"#{table_name}_in_batches_temp_table_#{apply_limits(self, start, finish, order).to_sql.hash.abs.to_s(36)}\"[(-63..)]}\")", "render_path": null, @@ -570,7 +570,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/account.rb", - "line": 1021, + "line": 1036, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "Account.find_by_sql(\"WITH RECURSIVE t AS (\\n SELECT * FROM #{Account.quoted_table_name} WHERE id=#{Shard.local_id_for(starting_account_id.parent_account_id).first}\\n UNION\\n SELECT accounts.* FROM #{Account.quoted_table_name} INNER JOIN t ON accounts.id=t.parent_account_id\\n)\\nSELECT * FROM t\\n\".squish)", "render_path": null, @@ -593,7 +593,7 @@ "check_name": "Redirect", "message": "Possible unprotected redirect", "file": "app/controllers/users_controller.rb", - "line": 2273, + "line": 2264, "link": "https://brakemanscanner.org/docs/warning_types/redirect/", "code": "redirect_to(MediaSourceFetcher.new(CanvasKaltura::ClientV3.new).fetch_preferred_source_url(:media_id => params[:entryId], :file_extension => ((params[:type] or params[:format])), :media_type => params[:media_type]))", "render_path": null, @@ -639,7 +639,7 @@ "check_name": "SSLVerify", "message": "SSL certificate verification was bypassed", "file": "gems/canvas_kaltura/lib/canvas_kaltura/kaltura_client_v3.rb", - "line": 393, + "line": 395, "link": "https://brakemanscanner.org/docs/warning_types/ssl_verification_bypass/", "code": "Net::HTTP.new(\"www.kaltura.com\", (if (CanvasKaltura::ClientV3.config[\"protocol\"] != \"http\") then\n Net::HTTP.https_default_port\nelse\n Net::HTTP.http_default_port\nend)).verify_mode = OpenSSL::SSL::VERIFY_NONE", "render_path": null, @@ -685,7 +685,7 @@ "check_name": "Redirect", "message": "Possible unprotected redirect", "file": "app/controllers/courses_controller.rb", - "line": 3406, + "line": 3407, "link": "https://brakemanscanner.org/docs/warning_types/redirect/", "code": "redirect_to((params[:continue_to] or course_url(@course)))", "render_path": null, @@ -708,7 +708,7 @@ "check_name": "SendFile", "message": "Model attribute used in file name", "file": "app/controllers/files_controller.rb", - "line": 1539, + "line": 1545, "link": "https://brakemanscanner.org/docs/warning_types/file_access/", "code": "send_file(Thumbnail.where(:id => params[:id], :uuid => params[:uuid]).first.full_filename, :content_type => Thumbnail.where(:id => params[:id], :uuid => params[:uuid]).first.content_type)", "render_path": null, @@ -844,7 +844,7 @@ "check_name": "Redirect", "message": "Possible unprotected redirect", "file": "app/controllers/users_controller.rb", - "line": 2463, + "line": 2454, "link": "https://brakemanscanner.org/docs/warning_types/redirect/", "code": "redirect_to(User.avatar_fallback_url(Shard.shard_for(Shard.global_id_for(User.user_id_from_avatar_key(params[:user_id]))).activate do\n Rails.cache.fetch(Cacher.avatar_cache_key(Shard.global_id_for(User.user_id_from_avatar_key(params[:user_id])), ((request.env[\"canvas.domain_root_account\"] or LoadAccount.default_domain_root_account).settings[:avatars] or \"enabled\"))) do\n user = User.where(:id => Shard.global_id_for(User.user_id_from_avatar_key(params[:user_id]))).first\nif User.where(:id => Shard.global_id_for(User.user_id_from_avatar_key(params[:user_id]))).first then\n User.where(:id => Shard.global_id_for(User.user_id_from_avatar_key(params[:user_id]))).first.avatar_url(nil, ((request.env[\"canvas.domain_root_account\"] or LoadAccount.default_domain_root_account).settings[:avatars] or \"enabled\"))\nelse\n \"/images/messages/avatar-50.png\"\nend\n end\n end, request))", "render_path": null, @@ -860,6 +860,29 @@ ], "note": "" }, + { + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "8ddd54b324d84f7db45c1f45295d77be2449745f9fdfc87bf3e2621302576aa7", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/controllers/smart_search_controller.rb", + "line": 67, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "WikiPage.find_by_sql([\" SELECT wp.*, (wpe.embedding #{quoted_operator_name(\"<=>\")} ?) AS distance\\n FROM #{WikiPage.quoted_table_name} wp\\n INNER JOIN #{Enrollment.quoted_table_name} AS e\\n ON wp.context_type = 'Course'\\n AND wp.context_id = e.course_id\\n INNER JOIN #{EnrollmentState.quoted_table_name} AS es\\n ON e.id = es.enrollment_id\\n INNER JOIN #{WikiPageEmbedding.quoted_table_name} AS wpe\\n ON wp.id = wpe.wiki_page_id\\n WHERE\\n e.user_id = ?\\n AND e.workflow_state <> 'deleted'\\n AND es.restricted_access = FALSE\\n AND es.state IN ('active', 'invited', 'pending_invited', 'pending_active')\\n AND e.type IN ('TeacherEnrollment', 'TaEnrollment', 'DesignerEnrollment', 'StudentViewEnrollment')\\n ORDER BY distance asc\\n LIMIT 25\\n\".squish, OpenAi.generate_embedding(params[:q])[0].to_s, ((((nil or PseudonymSession.find_with_validation.record) or Pseudonym.where(:id => (@policy_pseudonym_id)).first).user or nil) or api_find(User, session[:become_user_id])).id])", + "render_path": null, + "location": { + "type": "method", + "class": "SmartSearchController", + "method": "index" + }, + "user_input": "quoted_operator_name(\"<=>\")", + "confidence": "Medium", + "cwe_id": [ + 89 + ], + "note": "no user input is interpolated" + }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -890,7 +913,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/controllers/accounts_controller.rb", - "line": 1212, + "line": 1270, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "AccountReport.from(\"unnest('{#{AccountReport.available_reports.keys.join(\",\")}}'::text[]) report_types (name),\\n LATERAL (#{@account.account_reports.active.where(\"report_type=name\").most_recent.to_sql}) account_reports \")", "render_path": null, @@ -959,7 +982,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/assignment_override.rb", - "line": 310, + "line": 325, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "where(\"#{field}_overridden\" => true)", "render_path": null, @@ -982,7 +1005,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/controllers/conversations_controller.rb", - "line": 1064, + "line": 1069, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "InstStatsd::Statsd.count(\"inbox.conversation.unstarred.legacy\", params[:conversation_ids].length)", "render_path": null, @@ -1005,7 +1028,7 @@ "check_name": "UnsafeReflection", "message": "Unsafe reflection method `const_get` called with parameter value", "file": "app/controllers/application_controller.rb", - "line": 1429, + "line": 1464, "link": "https://brakemanscanner.org/docs/warning_types/remote_code_execution/", "code": "Object.const_get((params[:feed_code].split(\"_\", 2) or [\"group_membership\", params[:feed_code].split(\"_\", 3)[-1]])[0].classify, false)", "render_path": null, @@ -1028,7 +1051,7 @@ "check_name": "RegexDoS", "message": "Model attribute used in regular expression", "file": "app/models/course.rb", - "line": 2715, + "line": 2776, "link": "https://brakemanscanner.org/docs/warning_types/denial_of_service/", "code": "/\\A#{(Folder.root_folders(self).first.name + \"/\")}/", "render_path": null, @@ -1052,7 +1075,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/controllers/accounts_controller.rb", - "line": 1207, + "line": 1265, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "AccountReport.from(\"unnest('{#{AccountReport.available_reports.keys.join(\",\")}}'::text[]) report_types (name),\\n LATERAL (#{@account.account_reports.active.where(\"report_type=name\").most_recent.complete.to_sql}) account_reports \")", "render_path": null, @@ -1374,7 +1397,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/controllers/conversations_controller.rb", - "line": 1063, + "line": 1068, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "InstStatsd::Statsd.count(\"inbox.conversation.starred.legacy\", params[:conversation_ids].length)", "render_path": null, @@ -1397,7 +1420,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "config/initializers/active_record.rb", - "line": 881, + "line": 883, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "connection.execute(\"CLOSE #{cursor}\")", "render_path": null, @@ -1416,7 +1439,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/student_enrollment.rb", - "line": 88, + "line": 95, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "Submission.joins(:assignment).where(:user_id => students.map(&:user_id), :workflow_state => \"deleted\", :assignments => ({ :context_id => course_id })).merge(Assignment.active).in_batches.update_all(\"workflow_state = #{SubmissionLifecycleManager.infer_submission_workflow_state_sql}\")", "render_path": null, @@ -1439,7 +1462,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/controllers/conversations_controller.rb", - "line": 1065, + "line": 1070, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "InstStatsd::Statsd.count(\"inbox.conversation.unread.legacy\", params[:conversation_ids].length)", "render_path": null, @@ -1508,7 +1531,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "config/initializers/active_record.rb", - "line": 1029, + "line": 1031, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "connection.execute(\"ALTER TABLE #{\"#{table_name}_in_batches_temp_table_#{apply_limits(self, start, finish, order).to_sql.hash.abs.to_s(36)}\"[(-63..)]} ADD temp_primary_key SERIAL PRIMARY KEY\")", "render_path": null, @@ -1527,7 +1550,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "config/initializers/active_record.rb", - "line": 868, + "line": 870, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "connection.select_values(\"FETCH FORWARD #{of} FROM #{\"#{table_name}_in_batches_cursor_#{apply_limits(clone, start, finish, order).except(:select).select(primary_key).to_sql.hash.abs.to_s(36)}\"}\")", "render_path": null, @@ -1546,7 +1569,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "config/initializers/active_record.rb", - "line": 1579, + "line": 1581, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "connection.select_value(\"SELECT COUNT(*) FROM pg_proc WHERE proname='#{procname}'\")", "render_path": null, @@ -1661,7 +1684,7 @@ "check_name": "RegexDoS", "message": "Model attribute used in regular expression", "file": "app/models/content_migration.rb", - "line": 604, + "line": 603, "link": "https://brakemanscanner.org/docs/warning_types/denial_of_service/", "code": "/\\A#{(Folder.root_folders(context).first.name + \"/\")}/", "render_path": null, @@ -1702,6 +1725,6 @@ "note": "" } ], - "updated": "2023-06-23 20:34:01 +0000", - "brakeman_version": "5.4.1" + "updated": "2023-09-13 14:16:05 -0600", + "brakeman_version": "6.0.1" }