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

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Search, defect, P4)

Description

[krupa raj[:krupa]]

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


observed behavior:

Search is temporarily unavailable

server returns a 503 status

Comment 1

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

For the 3 people who search for "$" they can wait til we implement search in Zamboni

Comment 2

[krupa raj[:krupa]]

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

Comment 3

[krupa raj[:krupa]]

same problem with ^

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

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.

Comment 5

[krupa raj[:krupa]]

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.

Comment 6

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

There's no room for SQL injection.  It doesn't hit mysql.

Comment 7

[krupa raj[:krupa]]

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

Comment 8

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

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.

Comment 9

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

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.

Comment 10

[Wil Clouser [:clouserw]]

What's the status of this?

Comment 11

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

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

Comment 12

[Wil Clouser [:clouserw]]

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.

Comment 13

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

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.

Comment 14

[Wil Clouser [:clouserw]]

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.

Comment 15

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

I agree, we should stay on it -- I think it'll be useful for negative matching.

Comment 16

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

http://github.com/jbalogh/zamboni/commit/f28f77f0da762fbc9d0608a71d19dfd714c55067

Comment 17

[krupa raj[:krupa]]

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=