FIX: Prevent null-byte searches causing 500 error (#8226)

This fix ensures that searches that contain a null byte return a 400
error instead of causing a 500 error.

For some reason from rspec we will reach the raise statement inside
of the `rescue_from ArgumentError` block, but outside of rspec it will
not execute the raise statement and so a 500 is thrown instead of
reaching the `rescue_from Discourse::InvalidParameters` block inside of
the application controller.

This fix raises Discourse::InvalidParameters directly from the search
controller instead of relying on `PG::Connection.escape_string` to
raise the `ArgumentError`.
This commit is contained in:
Blake Erickson 2019-10-22 08:44:52 -06:00 committed by GitHub
parent 283a0add80
commit 7d09af7eda
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 15 additions and 0 deletions

View File

@ -24,6 +24,10 @@ class SearchController < ApplicationController
raise Discourse::InvalidParameters.new(:q)
end
if @search_term.present? && @search_term.include?("\u0000")
raise Discourse::InvalidParameters.new("string contains null byte")
end
search_args = {
type_filter: 'topic',
guardian: guardian,
@ -68,6 +72,10 @@ class SearchController < ApplicationController
def query
params.require(:term)
if params[:term].include?("\u0000")
raise Discourse::InvalidParameters.new("string contains null byte")
end
search_args = { guardian: guardian }
search_args[:type_filter] = params[:type_filter] if params[:type_filter].present?

View File

@ -197,6 +197,13 @@ describe SearchController do
expect(response.status).to eq(400)
end
it "returns a 400 error if you search for null bytes" do
term = "hello\0hello"
get "/search.json", params: { q: term }
expect(response.status).to eq(400)
end
it "logs the search term" do
SiteSetting.log_search_queries = true
get "/search.json", params: { q: 'bantha' }