JamesHeinrich
commented
1 year ago Thanks for the suggested fix. That code is, coincidentally, exactly 20 years old (v1.6.3, 2003-May-17), I'm not sure how it escaped being fixed for so long.
It looks like IsANumber is not needed at all with a minor tweak to IsValidDateStampString, but if I'm going to change it I'll leave it with the current set of parameters on the off-chance someone is calling that function elsewhere. Checking the codebase I see it's also currently used in write.id3v2.php lines 1053,1750 although those instances could easily be replaced if the function was removed entirely.
Alternate fix published in https://github.com/JamesHeinrich/getID3/commit/e3b69ad701422ac409739003fbdca01024b3ca46
kesselb
The old implementation, using a loop and chr, was incorrect.
$numberstring = '20230528';
$a = chr('2') will return an empty string. $b = chr('0') will return an empty string.
$a < $b is false.
This was reported by a Nextcloud user here: https://github.com/nextcloud/server/issues/37688
We run a fork version of getID3. I could reproduce the error with your library as well. The user provided the faulty mp3 file. There is apparently no ownership frame, but somehow this code path is executed and it fails when passing an invalid chracter to chr.
I assume the intention of the original implementation was to check if the character in the range 0-9.