enh(app/skin-selector): better DB intension through deferred FKs, further PNG validations

Author: AlexTMjugador

packages/app-lib/migrations/20250413162050_skin-selector.sql

@@ -15,7 +15,9 @@ CREATE TABLE custom_minecraft_skins (
 
     PRIMARY KEY (minecraft_user_uuid, texture_key, variant, cape_id),
     FOREIGN KEY (minecraft_user_uuid) REFERENCES minecraft_users(uuid)
-        ON DELETE CASCADE ON UPDATE CASCADE
+        ON DELETE CASCADE ON UPDATE CASCADE,
+    FOREIGN KEY (texture_key) REFERENCES custom_minecraft_skin_textures(texture_key)
+        ON DELETE CASCADE ON UPDATE CASCADE DEFERRABLE INITIALLY DEFERRED
 );
 
 CREATE TABLE custom_minecraft_skin_textures (
@@ -25,26 +27,10 @@ CREATE TABLE custom_minecraft_skin_textures (
     PRIMARY KEY (texture_key)
 );
 
--- Use triggers to emulate partial cascading foreign key constraints on the custom_minecraft_skin_textures table
-
-CREATE TRIGGER custom_minecraft_skin_texture_insertion_validation
-    BEFORE INSERT ON custom_minecraft_skin_textures FOR EACH ROW
-    BEGIN
-        SELECT CASE WHEN NOT EXISTS (
-            SELECT 1 FROM custom_minecraft_skins WHERE texture_key = NEW.texture_key
-        ) THEN RAISE(ABORT, 'Missing custom skin for the specified skin texture key') END;
-    END;
-
 CREATE TRIGGER custom_minecraft_skin_texture_delete_cleanup
     AFTER DELETE ON custom_minecraft_skins FOR EACH ROW
     BEGIN
         DELETE FROM custom_minecraft_skin_textures WHERE texture_key NOT IN (
             SELECT texture_key FROM custom_minecraft_skins
         );
     END;
-
-CREATE TRIGGER custom_minecraft_skin_texture_update_cleanup
-    AFTER UPDATE OF texture_key ON custom_minecraft_skins FOR EACH ROW
-    BEGIN
-        UPDATE custom_minecraft_skin_textures SET texture_key = NEW.texture_key WHERE texture_key = OLD.texture_key;
-    END;

packages/app-lib/src/api/minecraft_skins.rs

@@ -185,7 +185,7 @@ pub async fn get_available_skins() -> crate::Result<Vec<Skin>> {
                 // Several custom skins may reuse the same texture for different cape or skin model
                 // variations, so check all attributes for correctness
                 let is_equipped = !found_equipped_skin.load(Ordering::Acquire)
-                    && custom_skin.texture_key == &*current_skin.texture_key()
+                    && custom_skin.texture_key == *current_skin.texture_key()
                     && custom_skin.variant == current_skin.variant
                     && custom_skin.cape_id
                         == if custom_skin.cape_id.is_some() {
@@ -261,6 +261,11 @@ pub async fn add_and_equip_custom_skin(
     variant: MinecraftSkinVariant,
     cape_override: Option<Cape>,
 ) -> crate::Result<()> {
+    let (skin_width, skin_height) = png_dimensions(&texture_blob)?;
+    if skin_width != 64 || ![32, 64].contains(&skin_height) {
+        return Err(ErrorKind::InvalidSkinTexture)?;
+    }
+
     let cape_override = cape_override.map(|cape| cape.id);
     let state = State::get().await?;
 
@@ -270,7 +275,7 @@ pub async fn add_and_equip_custom_skin(
 
     // We have to equip the skin first, as it's the Mojang API backend who knows
     // how to compute the texture key we require, which we can then read from the
-    // updated player profile. This also ensures the skin data is indeed valid
+    // updated player profile
     mojang_api::MinecraftSkinOperation::equip(
         &selected_credentials,
         stream::iter([Ok::<_, String>(Bytes::clone(&texture_blob))]),
@@ -478,13 +483,15 @@ async fn sync_cape(
     Ok(())
 }
 
-fn texture_blob_to_data_url(texture_blob: Option<Vec<u8>>) -> Arc<Url> {
-    let data = texture_blob.map_or(
+fn texture_blob_to_data_url(texture_blob: Vec<u8>) -> Arc<Url> {
+    let data = if is_png(&texture_blob) {
+        Cow::Owned(texture_blob)
+    } else {
+        // Fall back to a placeholder texture if the DB somehow contains corrupt data
         Cow::Borrowed(
             &include_bytes!("minecraft_skins/assets/default/MissingNo.png")[..],
-        ),
-        Cow::Owned,
-    );
+        )
+    };
 
     Url::parse(&format!(
         "data:image/png;base64,{}",
@@ -493,3 +500,39 @@ fn texture_blob_to_data_url(texture_blob: Option<Vec<u8>>) -> Arc<Url> {
     .unwrap()
     .into()
 }
+
+fn is_png(data: &[u8]) -> bool {
+    /// The initial 8 bytes of a PNG file, used to identify it as such.
+    ///
+    /// Reference: <https://www.w3.org/TR/png-3/#3PNGsignature>
+    const PNG_SIGNATURE: &[u8] =
+        &[0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A];
+
+    data.starts_with(PNG_SIGNATURE)
+}
+
+fn png_dimensions(data: &[u8]) -> crate::Result<(u32, u32)> {
+    if !is_png(data) {
+        Err(ErrorKind::InvalidPng)?;
+    }
+
+    // Read the width and height fields from the IHDR chunk, which the
+    // PNG specification mandates to be the first in the file, just after
+    // the 8 signature bytes. See:
+    // https://www.w3.org/TR/png-3/#5DataRep
+    // https://www.w3.org/TR/png-3/#11IHDR
+    let width = u32::from_be_bytes(
+        data.get(16..20)
+            .ok_or(ErrorKind::InvalidPng)?
+            .try_into()
+            .unwrap(),
+    );
+    let height = u32::from_be_bytes(
+        data.get(20..24)
+            .ok_or(ErrorKind::InvalidPng)?
+            .try_into()
+            .unwrap(),
+    );
+
+    Ok((width, height))
+}

packages/app-lib/src/error.rs

@@ -128,6 +128,14 @@ pub enum ErrorKind {
 
     #[error("Invalid data URL: {0}")]
     InvalidDataUrlBase64(#[from] data_url::forgiving_base64::InvalidBase64),
+
+    #[error("Invalid PNG")]
+    InvalidPng,
+
+    #[error(
+        "A skin texture must have a dimension of either 64x64 or 64x32 pixels"
+    )]
+    InvalidSkinTexture,
 }
 
 #[derive(Debug)]

packages/app-lib/src/state/minecraft_skins/mod.rs

@@ -150,12 +150,12 @@ impl CustomMinecraftSkin {
     pub async fn texture_blob(
         &self,
         db: impl sqlx::Acquire<'_, Database = sqlx::Sqlite>,
-    ) -> crate::Result<Option<Vec<u8>>> {
+    ) -> crate::Result<Vec<u8>> {
         Ok(sqlx::query_scalar!(
             "SELECT texture FROM custom_minecraft_skin_textures WHERE texture_key = ?",
             self.texture_key
         )
-        .fetch_optional(&mut *db.acquire().await?)
+        .fetch_one(&mut *db.acquire().await?)
         .await?)
     }