Skip to content

Commit

Permalink
fix sql injection on upload_handle
Browse files Browse the repository at this point in the history
  • Loading branch information
itismadness authored and itismadness committed Sep 16, 2019
1 parent 06d23c8 commit 6844e15
Showing 1 changed file with 14 additions and 13 deletions.
27 changes: 14 additions & 13 deletions sections/upload/upload_handle.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,27 +330,28 @@
$ArtistForm = array();
}
$LogName .= Artists::display_artists($ArtistForm, false, true, false);
} elseif ($Type == 'Music' && empty($ArtistForm)) {
$DB->query("
}

if ($Err) { // Show the upload form, with the data the user entered
$UploadForm = $Type;
include(SERVER_ROOT.'/sections/upload/upload.php');
die();
}

if (!empty($Properties['GroupID']) && empty($ArtistForm) && $Type == 'Music') {
$DB->prepared_query("
SELECT ta.ArtistID, aa.Name, ta.Importance
FROM torrents_artists AS ta
JOIN artists_alias AS aa ON ta.AliasID = aa.AliasID
WHERE ta.GroupID = ".$Properties['GroupID']."
ORDER BY ta.Importance ASC, aa.Name ASC;");
WHERE ta.GroupID = ?
ORDER BY ta.Importance ASC, aa.Name ASC;", $Properties['GroupID']);
while (list($ArtistID, $ArtistName, $ArtistImportance) = $DB->next_record(MYSQLI_BOTH, false)) {
$ArtistForm[$ArtistImportance][] = array('id' => $ArtistID, 'name' => display_str($ArtistName));
$ArtistsUnescaped[$ArtistImportance][] = array('name' => $ArtistName);
}
$LogName .= Artists::display_artists($ArtistsUnescaped, false, true, false);
}


if ($Err) { // Show the upload form, with the data the user entered
$UploadForm = $Type;
include(SERVER_ROOT.'/sections/upload/upload.php');
die();
}

// Strip out Amazon's padding
$AmazonReg = '/(http:\/\/ecx.images-amazon.com\/images\/.+)(\._.*_\.jpg)/i';
$Matches = array();
Expand Down Expand Up @@ -780,12 +781,12 @@
INSERT INTO torrents
(GroupID, UserID, Media, Format, Encoding,
Remastered, RemasterYear, RemasterTitle, RemasterRecordLabel, RemasterCatalogueNumber,
Scene, HasLog, HasCue, HasLogDB, LogScore, LogChecksum, info_hash, FileCount, FileList,
Scene, HasLog, HasCue, HasLogDB, LogScore, LogChecksum, info_hash, FileCount, FileList,
FilePath, Size, Time, Description, FreeTorrent, FreeLeechType)
VALUES
($GroupID, $LoggedUser[ID], $T[Media], $T[Format], $T[Encoding],
$T[Remastered], $T[RemasterYear], $T[RemasterTitle], $T[RemasterRecordLabel], $T[RemasterCatalogueNumber],
$T[Scene], '$HasLog', '$HasCue', '$LogInDB', '$LogScore', '$LogChecksum','".db_string($InfoHash)."', $NumFiles, '$FileString',
$T[Scene], '$HasLog', '$HasCue', '$LogInDB', '$LogScore', '$LogChecksum','".db_string($InfoHash)."', $NumFiles, '$FileString',
'$FilePath', $TotalSize, '".sqltime()."', $T[TorrentDescription], '$T[FreeLeech]', '$T[FreeLeechType]')");

$Cache->increment('stats_torrent_count');
Expand Down

4 comments on commit 6844e15

@neobenedict
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How was this vulnerable? The data was validated on line 184

@itismadness
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 184 only sets how that particular variable will get validated, it does not actually kill the script or anything. However, on line 248, the data is finally validated, but it just returns an error string that validation failed. This would be fine if the error was immediately displayed and the script bailed, but it did not, and continued, running a SQL query with the data that failed validation before we would see the error screen:

Gazelle/sections/upload/upload_handle.php

Lines 333 to 352 in 06d23c8

} elseif ($Type == 'Music' && empty($ArtistForm)) {
$DB->query("
SELECT ta.ArtistID, aa.Name, ta.Importance
FROM torrents_artists AS ta
JOIN artists_alias AS aa ON ta.AliasID = aa.AliasID
WHERE ta.GroupID = ".$Properties['GroupID']."
ORDER BY ta.Importance ASC, aa.Name ASC;");
while (list($ArtistID, $ArtistName, $ArtistImportance) = $DB->next_record(MYSQLI_BOTH, false)) {
$ArtistForm[$ArtistImportance][] = array('id' => $ArtistID, 'name' => display_str($ArtistName));
$ArtistsUnescaped[$ArtistImportance][] = array('name' => $ArtistName);
}
$LogName .= Artists::display_artists($ArtistsUnescaped, false, true, false);
}
if ($Err) { // Show the upload form, with the data the user entered
$UploadForm = $Type;
include(SERVER_ROOT.'/sections/upload/upload.php');
die();
}

This is compounded by the fact the variable was not properly escaped when passed into the DB so could easily be a malicious payload. Let me know if this makes sense.

@neobenedict
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, that is unfortunate

There are probably a lot of other sites still vulnerable to this.

@itismadness
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that they are all closed source, it's impossible to say.

Please sign in to comment.