Mixxx

Search queries not properly escaped.

  1. Series 1.9
  2. Bug #768022
Bug #768022 reported by Owen Williams
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Mixxx
Fix Released
Medium
RJ Skerry-Ryan
1.10
Fix Released
Medium
RJ Skerry-Ryan
Mixxx 1.10.0
1.9
Fix Committed
Medium
RJ Skerry-Ryan
Mixxx 1.9.3

Bug Description

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"

Revision history for this message
William Good (bkgood) wrote :

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.

Revision history for this message
Owen Williams (ywwg) wrote :

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.

Revision history for this message
William Good (bkgood) wrote :

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).

RJ Skerry-Ryan (rryan)
Changed in mixxx:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

Whoops, fixed this in Bug #905776

Changed in mixxx:
assignee: nobody → RJ Ryan (rryan)
status: Confirmed → Fix Committed
security vulnerability: yes → no
visibility: private → public
summary: - sql bug in library search code (possible injection attacks possible?)
+ Search queries not properly escaped.
RJ Skerry-Ryan (rryan)
Changed in mixxx:
status: Fix Committed → Fix Released
Revision history for this message
mangelasakis (mangelasakis) wrote :

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.

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

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?

Revision history for this message
Swiftb0y (swiftb0y) wrote :

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/5870

lock status: Metadata changes locked and limited to project staff
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.