Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search queries not properly escaped. #5870

Closed
mixxxbot opened this issue Aug 22, 2022 · 7 comments
Closed

Search queries not properly escaped. #5870

mixxxbot opened this issue Aug 22, 2022 · 7 comments
Labels

Comments

@mixxxbot
Copy link
Collaborator

Reported by: ywwg
Date: 2011-04-21T03:26:38Z
Status: Fix Released
Importance: Medium
Launchpad Issue: lp768022


Search terms beginning with numbers cause problems in the query-building code of basesqltablemodel. In my case, I have a folder called "00MIXING" where I put all my mixing tracks. Searching for "00MIXING" fails, because the following query is built:

"SELECT id FROM library_view WHERE ((artist LIKE 'ORDER BY lower(library_view."datetime_added") DESCmixi%' OR album LIKE 'ORDER BY lower(library_view."datetime_added") DESCmixi%' OR location LIKE 'ORDER BY lower(library_view."datetime_added") DESCmixi%' OR comment LIKE 'ORDER BY lower(library_view."datetime_added") DESCmixi%' OR title LIKE 'ORDER BY lower(library_view."datetime_added") DESCmixi%')) %4"

What's happening is the zeros at the head of the string become "%00MIXING" and then calls to QString .arg() interpret the %0 as an argument to be replaced.  If I were to search for "0; drop table whatever;", this could be a security problem.

For example, a proper search string, using just "MIXING", yields:

"SELECT id FROM library_view WHERE ((artist LIKE '%mixing%' OR album LIKE '%mixing%' OR location LIKE '%mixing%' OR comment LIKE '%mixing%' OR title LIKE '%mixing%')) ORDER BY lower(library_view."datetime_added") DESC"

@mixxxbot
Copy link
Collaborator Author

Commented by: bkgood
Date: 2011-04-21T06:46:45Z


This really isn't a security issue I don't think. Remote SQL injection is always a concern on web-based projects but a user can screw up their own local database in mixxx any number of ways, including rm ~/.mixxx/mixxxdb.sqlite (if they want to be efficient about it). A user exploiting this does nothing to harm anyone but themselves.

That said, this does appear to be a bug in its own right.

@mixxxbot mixxxbot added the bug label Aug 22, 2022
@mixxxbot
Copy link
Collaborator Author

Commented by: ywwg
Date: 2011-04-21T11:59:43Z


My thinking was that DJ Laptops tend to be in public areas, and I could imagine some smartass waiting until the laptop is unattended and then typing in a search query that destroys the DJ's library.

@mixxxbot
Copy link
Collaborator Author

Commented by: bkgood
Date: 2011-04-30T03:43:16Z


Could this be fixed by just subbing in the search term after the .arg calls are made? Also, this is a bit weird as the qt docs (http://doc.qt.nokia.com/latest/qstring.html#arg) say the markers must be in %[1-99], i.e., not including zero, so I'm not sure why QString::arg is subbing it out, but this bug will still be triggered by other strings beginning with numerals (if I'm understanding this correctly).

@mixxxbot
Copy link
Collaborator Author

Commented by: rryan
Date: 2011-12-18T18:52:41Z


Whoops, fixed this in Bug #⁠905776

@mixxxbot
Copy link
Collaborator Author

Commented by: mangelasakis
Date: 2012-02-13T14:02:02Z


There is still a serious bug. The mixxx does not import all mp3 files in the library. Perhaps this is related to the bug here and I mention this in the same section.

@mixxxbot
Copy link
Collaborator Author

Commented by: rryan
Date: 2012-02-13T15:19:08Z


Hi mangelasakis,

It's not likely the two issues are related given that this bug refers to issues when using the search feature and your problem would be related to the library scanner. Could you file a separate bug please?

@mixxxbot
Copy link
Collaborator Author

Issue closed with status Fix Released.

@mixxxbot mixxxbot transferred this issue from another repository Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant