Searching for $ and ^ shows "search is temporarily unavailable"

VERIFIED FIXED in 5.11

Status

addons.mozilla.org Graveyard
Search
P4
minor
VERIFIED FIXED
8 years ago
2 years ago

People

(Reporter: krupa, Assigned: davedash)

Tracking

unspecified
5.11

Details

(Whiteboard: [z][r?clouserw][gh:davedash/550046-bad-chars], URL)

(Reporter)

Description

8 years ago
steps to reproduce:
1.Go to preview homepage
2.Search for "$"


observed behavior:

Search is temporarily unavailable

server returns a 503 status
For the 3 people who search for "$" they can wait til we implement search in Zamboni
Priority: -- → P5
Target Milestone: 5.8 → 4.x (triaged)
(Reporter)

Comment 2

8 years ago
searching for the same in the API shows-<error>Could not connect to Sphinx search.</error>


https://preview.addons.mozilla.org/en-US/firefox/api/1.3/search/%24
Priority: P5 → --
Target Milestone: 4.x (triaged) → 5.8
(Reporter)

Updated

8 years ago
Target Milestone: 5.8 → ---
(Reporter)

Comment 3

8 years ago
same problem with ^
(Reporter)

Updated

8 years ago
Severity: normal → critical
Summary: Searching for $ shows "search is temporarily unavailable" → Searching for $ and ^ shows "search is temporarily unavailable"
Whiteboard: [sg:?]
How is this a security bug?  https://preview.addons.mozilla.org/en-US/firefox/search?q=foo%24&cat=all still works, so it seems limited to these two characters by themselves; just trying to understand the scope of this.
(Reporter)

Comment 5

8 years ago
Using $ and ^ in search,search becomes unavailable.This may open a window for SQL injection.I just want to confirm that this is NOT a security issue.So having the experts confirm it.
There's no room for SQL injection.  It doesn't hit mysql.
(Reporter)

Comment 7

8 years ago
spoke to James and Dave and I am now convinced that this indeed not a security issue.

However we should consider changing the error message to "No results found" which is less alarming than service unavailable
Severity: critical → minor
Whiteboard: [sg:?]
SearchError: index collections: expression 'addon_id' must be aliased (use 'expr AS alias' syntax); index addons: syntax error, unexpected '$' near '$'

This is odd... it doesn't like looking for just the end of a string... but it never hits the query parser... this gets stuck in the sphinxapi.py.
Review once we're unfrozen for 5.9.

Note: I may add more stuff to this branch, each commit I'll try to delineate with bug numbers.
Status: NEW → ASSIGNED
Whiteboard: [z][r?clouserw][gh:davedash/search-bugs]
Target Milestone: --- → 5.9
Assignee: nobody → dd
Priority: -- → P4
Target Milestone: 5.9 → 5.10
What's the status of this?
Target Milestone: 5.10 → 5.11
Whiteboard: [z][r?clouserw][gh:davedash/search-bugs] → [z][r?clouserw][gh:davedash/550046-bad-chars]
It was in search-bugs, but I must have deleted it thinking I had merged it into master... guess not.  Redid this again here:

550046-bad-chars
are there official sphinx docs that say queries with $ or ^ on the end are the only things sphinx can't handle and there is no way to escape them?  I'm wary of blacklists.
In http://www.sphinxsearch.com/docs/current.html
Under 4.3. Extended query syntax

They are listed as modifiers (begining/end of fields).  We don't use extended syntax now.  

We can modify this to do a more extensive cleansing where we remove any strings such as \w+\$.*\^\w+ - but I was hoping to avoid complicated regular expressions.

I'd leave it this way, until we decided to document to end-users extended syntax.
We aren't using any part of extended syntax?  Not even phrase searching?  Would it be better to switch the matching mode to SPH_MATCH_ALL?  For the record, I'd rather stay on extended syntax and support at least match phrase and boolean at some point.

Anyway, r+. thanks.
I agree, we should stay on it -- I think it'll be useful for negative matching.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 17

8 years ago
no more error messages when searching for $ or ^-
https://preview.addons.mozilla.org/en-US/firefox/search/?q=%24&cat=all&lver=any&atype=0&sort=&pid=1&pp=20&lup=&advanced=
Status: RESOLVED → VERIFIED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.