From ce5e2f2392b5680a40b88b28e88b7c9c5cb0587a Mon Sep 17 00:00:00 2001
From: Domenico Cerasuolo <cerasuolodomenico@gmail.com>
Date: Wed, 3 Jan 2024 11:04:49 +0100
Subject: [PATCH] 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.
---
 core/src/spotify_id.rs | 41 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/core/src/spotify_id.rs b/core/src/spotify_id.rs
index 9b2d78fd..959b84ee 100644
--- a/core/src/spotify_id.rs
+++ b/core/src/spotify_id.rs
@@ -98,6 +98,9 @@ impl SpotifyId {
     ///
     /// [Spotify ID]: https://developer.spotify.com/documentation/web-api/concepts/spotify-uris-ids
     pub fn from_base16(src: &str) -> SpotifyIdResult {
+        if src.len() != 32 {
+            return Err(SpotifyIdError::InvalidId.into());
+        }
         let mut dst: u128 = 0;
 
         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
     pub fn from_base62(src: &str) -> SpotifyIdResult {
+        if src.len() != 22 {
+            return Err(SpotifyIdError::InvalidId.into());
+        }
         let mut dst: u128 = 0;
 
         for c in src.as_bytes() {
@@ -133,8 +139,8 @@ impl SpotifyId {
                 _ => return Err(SpotifyIdError::InvalidId.into()),
             } as u128;
 
-            dst *= 62;
-            dst += p;
+            dst = dst.checked_mul(62).ok_or(SpotifyIdError::InvalidId)?;
+            dst = dst.checked_add(p).ok_or(SpotifyIdError::InvalidId)?;
         }
 
         Ok(Self {
@@ -606,7 +612,7 @@ mod tests {
         },
     ];
 
-    static CONV_INVALID: [ConversionCase; 3] = [
+    static CONV_INVALID: [ConversionCase; 5] = [
         ConversionCase {
             id: 0,
             kind: SpotifyItemType::Unknown,
@@ -631,13 +637,40 @@ mod tests {
                 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 {
             id: 0,
             kind: SpotifyItemType::Unknown,
             // Uri too short
             uri: "spotify:azb:aRS48xBl0tH",
             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: &[
                 // Invalid length.
                 154, 27, 28, 251,