Fix SpotifyId base 62 and 16 str decoding

A SpotifyId is expected to be a 128 bits integer and can be parsed from a
base 62 or 16 string. However the parsing functions only checked the validity of
the characters of the string, but not its length. This could result in integer
overflows or the parsing of incorrect strings as Spotify ids.

This commit add some checks to the length of the input string passed to the
parse functions, and also checks for integer overflows in case of base62
encoded strings.
This commit is contained in:
Domenico Cerasuolo 2024-01-03 11:04:49 +01:00
parent c37ec8c859
commit ce5e2f2392

View file

@ -98,6 +98,9 @@ impl SpotifyId {
/// ///
/// [Spotify ID]: https://developer.spotify.com/documentation/web-api/concepts/spotify-uris-ids /// [Spotify ID]: https://developer.spotify.com/documentation/web-api/concepts/spotify-uris-ids
pub fn from_base16(src: &str) -> SpotifyIdResult { pub fn from_base16(src: &str) -> SpotifyIdResult {
if src.len() != 32 {
return Err(SpotifyIdError::InvalidId.into());
}
let mut dst: u128 = 0; let mut dst: u128 = 0;
for c in src.as_bytes() { for c in src.as_bytes() {
@ -123,6 +126,9 @@ impl SpotifyId {
/// ///
/// [Spotify ID]: https://developer.spotify.com/documentation/web-api/concepts/spotify-uris-ids /// [Spotify ID]: https://developer.spotify.com/documentation/web-api/concepts/spotify-uris-ids
pub fn from_base62(src: &str) -> SpotifyIdResult { pub fn from_base62(src: &str) -> SpotifyIdResult {
if src.len() != 22 {
return Err(SpotifyIdError::InvalidId.into());
}
let mut dst: u128 = 0; let mut dst: u128 = 0;
for c in src.as_bytes() { for c in src.as_bytes() {
@ -133,8 +139,8 @@ impl SpotifyId {
_ => return Err(SpotifyIdError::InvalidId.into()), _ => return Err(SpotifyIdError::InvalidId.into()),
} as u128; } as u128;
dst *= 62; dst = dst.checked_mul(62).ok_or(SpotifyIdError::InvalidId)?;
dst += p; dst = dst.checked_add(p).ok_or(SpotifyIdError::InvalidId)?;
} }
Ok(Self { Ok(Self {
@ -606,7 +612,7 @@ mod tests {
}, },
]; ];
static CONV_INVALID: [ConversionCase; 3] = [ static CONV_INVALID: [ConversionCase; 5] = [
ConversionCase { ConversionCase {
id: 0, id: 0,
kind: SpotifyItemType::Unknown, kind: SpotifyItemType::Unknown,
@ -631,13 +637,40 @@ mod tests {
154, 27, 28, 251, 154, 27, 28, 251,
], ],
}, },
ConversionCase {
id: 0,
kind: SpotifyItemType::Unknown,
// Uri too short
uri: "spotify:azb:aRS48xBl0tH",
// too long, should return error but not panic overflow
base16: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
// too long, should return error but not panic overflow
base62: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
raw: &[
// Invalid length.
154, 27, 28, 251,
],
},
ConversionCase { ConversionCase {
id: 0, id: 0,
kind: SpotifyItemType::Unknown, kind: SpotifyItemType::Unknown,
// Uri too short // Uri too short
uri: "spotify:azb:aRS48xBl0tH", uri: "spotify:azb:aRS48xBl0tH",
base16: "--------------------", base16: "--------------------",
base62: "....................", // too short to encode a 128 bits int
base62: "aa",
raw: &[
// Invalid length.
154, 27, 28, 251,
],
},
ConversionCase {
id: 0,
kind: SpotifyItemType::Unknown,
uri: "cleary invalid uri",
base16: "--------------------",
// too high of a value, this would need a 132 bits int
base62: "ZZZZZZZZZZZZZZZZZZZZZZ",
raw: &[ raw: &[
// Invalid length. // Invalid length.
154, 27, 28, 251, 154, 27, 28, 251,