Skip to content

Commit

Permalink
Avoid overflow on numeric DB fields while scanning audio files
Browse files Browse the repository at this point in the history
There are a few integer fields stored to the database which get their
values from the data scanned from the input files: disk, number,
year, length, bitrate.

As we have no control over what kind of data may be scanned from the
files, the values need to be sanitized before entering them to the DB.
If the value wouldn't fit in the numeric limits of the DB field in
question, there would be an unhandled exception and that would break
the scanning process.

refs owncloud#1073
  • Loading branch information
paulijar committed Jul 20, 2023
1 parent f52c019 commit 2a0431b
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 13 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
### Fixed
- Subsonic: Unhandled exception when attempting to delete a non-existent bookmark
[#1071](https://github.com/owncloud/music/issues/1071)
- Scanning breaking if any out-of-bounds numeric value gets scanned from any audio file
[#1073](https://github.com/owncloud/music/issues/1073)

## 1.8.4 - 2023-06-06
### Added
Expand Down
33 changes: 21 additions & 12 deletions lib/Utility/Scanner.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* @author Morris Jobke <hey@morrisjobke.de>
* @author Pauli Järvinen <pauli.jarvinen@gmail.com>
* @copyright Morris Jobke 2013, 2014
* @copyright Pauli Järvinen 2016 - 2021
* @copyright Pauli Järvinen 2016 - 2023
*/

namespace OCA\Music\Utility;
Expand Down Expand Up @@ -245,12 +245,9 @@ private function extractMetadata(File $file, Folder $libraryRoot, string $filePa

$meta['picture'] = ExtractorGetID3::getTag($fileInfo, 'picture', true);

$meta['length'] = $fileInfo['playtime_seconds'] ?? null;
if ($meta['length'] !== null) {
$meta['length'] = \round($meta['length']);
}
$meta['length'] = self::normalizeUnsigned($fileInfo['playtime_seconds'] ?? null);

$meta['bitrate'] = $fileInfo['audio']['bitrate'] ?? null;
$meta['bitrate'] = self::normalizeUnsigned($fileInfo['audio']['bitrate'] ?? null);

return $meta;
}
Expand Down Expand Up @@ -684,7 +681,7 @@ private function userL10N(string $userId) : IL10N {
return $this->l10nFactory->get('music', $languageCode);
}

private static function normalizeOrdinal($ordinal) : ?int {
private static function normalizeOrdinal(/*mixed*/ $ordinal) : ?int {
if (\is_string($ordinal)) {
// convert format '1/10' to '1'
$ordinal = \explode('/', $ordinal)[0];
Expand All @@ -697,7 +694,7 @@ private static function normalizeOrdinal($ordinal) : ?int {
$ordinal = null;
}

return $ordinal;
return Util::limit($ordinal, 0, Util::UINT32_MAX);
}

private static function parseFileName(string $fileName) : array {
Expand All @@ -713,16 +710,28 @@ private static function parseFileName(string $fileName) : array {
}
}

private static function normalizeYear($date) : ?int {
private static function normalizeYear(/*mixed*/ $date) : ?int {
$year = null;
$matches = null;

if (\ctype_digit($date)) {
return (int)$date; // the date is a valid year as-is
$year = (int)$date; // the date is a valid year as-is
} elseif (\is_string($date) && \preg_match('/^(\d\d\d\d)-\d\d-\d\d.*/', $date, $matches) === 1) {
return (int)$matches[1]; // year from ISO-formatted date yyyy-mm-dd
$year = (int)$matches[1]; // year from ISO-formatted date yyyy-mm-dd
} else {
$year = null;
}

return Util::limit($year, Util::SINT32_MIN, Util::SINT32_MAX);
}

private static function normalizeUnsigned(/*mixed*/ $value) : ?int {
if (\is_numeric($value)) {
$value = (int)\round((float)$value);
} else {
return null;
$value = null;
}
return Util::limit($value, 0, Util::UINT32_MAX);
}

/**
Expand Down
18 changes: 17 additions & 1 deletion lib/Utility/Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* later. See the COPYING file.
*
* @author Pauli Järvinen <pauli.jarvinen@gmail.com>
* @copyright Pauli Järvinen 2018 - 2021
* @copyright Pauli Järvinen 2018 - 2023
*/

namespace OCA\Music\Utility;
Expand All @@ -19,6 +19,10 @@
*/
class Util {

const UINT32_MAX = 0xFFFFFFFF;
const SINT32_MAX = 0x7FFFFFFF;
const SINT32_MIN = -self::SINT32_MAX - 1;

/**
* Map the given array by calling a named member function for each of the array elements
*/
Expand Down Expand Up @@ -371,4 +375,16 @@ public static function swap(&$a, &$b) : void {
$a = $b;
$b = $temp;
}

/**
* Limit an integer value between the specified minimum and maximum.
* A null value is a valid input and will produce a null output.
*/
public static function limit(?int $input, int $min, int $max) : ?int {
if ($input === null) {
return null;
} else {
return \max($min, \min($input, $max));
}
}
}

0 comments on commit 2a0431b

Please sign in to comment.