search

flarum / issue-archive

0 stars 0 forks source link

Missing LIKE escaping on search string in User/Search/Gambit/FulltextGambit #111

Open AlexanderOMara opened 3 years ago

AlexanderOMara commented 3 years ago

Bug Report

Current Behavior Currently if I do a search for %%% in the search bar, it matches all the users. This is because % and _ are wildcards in the LIKE queries.

Steps to Reproduce

  1. Go to 'https://discuss.flarum.org/'
  2. Search for %%%
  3. Wait a second and see a list of the first 5 users that matched.

Expected Behavior I wouldn't expect any users to match.

Screenshots

pngout

Environment

$ php flarum info
Flarum core 0.1.0-beta.16
PHP version: 7.4.16
Loaded extensions: Core, date, libxml, openssl, pcre, sqlite3, zlib, ctype, curl, dom, fileinfo, filter, ftp, hash, iconv, json, mbstring, SPL, PDO, session, posix, readline, Reflection, standard, SimpleXML, pdo_sqlite, Phar, tokenizer, xml, xmlreader, xmlwriter, mysqlnd, bcmath, exif, gd, mysqli, pdo_mysql, sodium, zip, Zend OPcache
+-------------------------+------------------+------------------------------------------+
| Flarum Extensions       |                  |                                          |
+-------------------------+------------------+------------------------------------------+
| ID                      | Version          | Commit                                   |
+-------------------------+------------------+------------------------------------------+
| flarum-flags            | v0.1.0-beta.16   |                                          |
| flarum-tags             | v0.1.0-beta.16   |                                          |
| flarum-suspend          | v0.1.0-beta.16   |                                          |
| flarum-subscriptions    | v0.1.0-beta.16   |                                          |
| flarum-sticky           | v0.1.0-beta.16   |                                          |
| flarum-statistics       | v0.1.0-beta.16   |                                          |
| flarum-mentions         | v0.1.0-beta.16   |                                          |
| flarum-markdown         | v0.1.0-beta.16.1 |                                          |
| flarum-lock             | v0.1.0-beta.16   |                                          |
| flarum-likes            | v0.1.0-beta.16   |                                          |
| flarum-lang-english     | v0.1.0-beta.16   |                                          |
| flarum-emoji            | v0.1.0-beta.16   |                                          |
| flarum-bbcode           | v0.1.0-beta.16   |                                          |
| flarum-approval         | v0.1.0-beta.16   |                                          |
+-------------------------+------------------+------------------------------------------+
Base URL: https://localhost/forum
Installation path: /var/www/html/forum
Debug mode: ON
Don't forget to turn off debug mode! It should never be turned on in a production system.

Possible Solution See these functions where the string simply has the % appended without escaping the string first. https://github.com/flarum/core/blob/023871ef86d436cc14631ee63cbbfd3ef9fd4bf1/src/User/Search/Gambit/FulltextGambit.php#L35-L53

These functions on the other hand do escape the value first. https://github.com/flarum/core/blob/46794483005b03455ceb128acfd057c26fa4639f/src/User/UserRepository.php#L106-L142

askvortsov1 commented 3 years ago

Hmm, interesting! I'm assuming that a prepared statement was used in the latter example because we're using raw methods (as we should be!!!! Not escaping there would be very bad...). Not sure we can (or need to) use prepared statements with non-raw Eloquent methods, so if we were escaping, it'd be manual.

Do we want to allow wildcards in user searches? That could have conceivable use cases, I suppose?

AlexanderOMara commented 3 years ago

There's a lot of wrong information on the internet about prepare statements when it comes to LIKE expressions. AFAIK, you actually can't use a prepare statement to get around having to escape the wildcard characters in the string, because LIKE operates on the string data itself.

askvortsov1 commented 3 years ago

Ah my bad, I missed the $string = $this->escapeLikeString($string); line :facepalm:

Welp since we have a solution, if we want to change this behavior, I suppose we could just copy over the method. I'm not sure that we'd want to create a whole util class just for this.

mmm8955405 commented 3 years ago

Is there precompiled, such as select * from table where a = "?", And it prevents SQL injection？？

mmm8955405 commented 3 years ago

If you filter or replace the characters entered by the user, it may also be regarded as an SQL search statement error. You prefer to encounter special characters and directly return zero records