From 5901b79d680570a4d4fcab8d9649524cef3bbba7 Mon Sep 17 00:00:00 2001 From: Purdze Date: Sat, 19 Sep 2026 21:36:50 +0100 Subject: [PATCH 1/4] only fetch skin textures from textures.minecraft.net --- pomme-client/src/renderer/mod.rs | 66 +++++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 6 deletions(-) diff --git a/pomme-client/src/renderer/mod.rs b/pomme-client/src/renderer/mod.rs index 1a972541d..8b2d3a61f 100644 --- a/pomme-client/src/renderer/mod.rs +++ b/pomme-client/src/renderer/mod.rs @@ -2109,14 +2109,32 @@ fn skin_url_from_texture_property(value: &str) -> Result<(String, bool), String> .map_err(error_chain)?; let payload: TexturesPayload = serde_json::from_slice(&decoded).map_err(error_chain)?; - payload + let skin = payload .textures .skin - .map(|s| { - let slim = s.metadata.as_ref().and_then(|m| m.model.as_deref()) == Some("slim"); - (s.url, slim) - }) - .ok_or_else(|| "No skin texture".to_string()) + .ok_or_else(|| "No skin texture".to_string())?; + if !is_allowed_texture_url(&skin.url) { + return Err(format!("texture url not allowed: {}", skin.url)); + } + let slim = skin.metadata.as_ref().and_then(|m| m.model.as_deref()) == Some("slim"); + Ok((skin.url, slim)) +} + +/// authlib's `TextureUrlChecker.isAllowedTextureDomain`: http(s) on exactly +/// `textures.minecraft.net`. Scheme and host are checked on the raw text since +/// `Url` lowercases both and authlib is case-sensitive. +fn is_allowed_texture_url(url: &str) -> bool { + const HOST: &str = "textures.minecraft.net"; + let Ok(parsed) = reqwest::Url::parse(url) else { + return false; + }; + let Some((scheme, rest)) = url.split_once("://") else { + return false; + }; + let authority = rest.split(['/', '?', '#']).next().unwrap_or_default(); + let host_port = authority.rsplit_once('@').map_or(authority, |(_, h)| h); + let host = host_port.split_once(':').map_or(host_port, |(h, _)| h); + matches!(scheme, "http" | "https") && host == HOST && parsed.host_str() == Some(HOST) } /// Error message including the source chain (`reqwest` hides the detail there). @@ -2356,6 +2374,42 @@ mod tests { ); } + #[test] + fn texture_urls_follow_authlib_allow_list() { + for ok in [ + "https://textures.minecraft.net/texture/abc", + "http://textures.minecraft.net/texture/abc", + "https://textures.minecraft.net:443/texture/abc", + ] { + assert!(is_allowed_texture_url(ok), "{ok}"); + } + for bad in [ + "https://example.com/texture/abc", + "https://textures.minecraft.net.evil.com/abc", + "https://evil.com/textures.minecraft.net", + "https://textures.minecraft.net@evil.com/abc", + "https://evil.com#@textures.minecraft.net", + "https://TEXTURES.minecraft.net/texture/abc", + "HTTPS://textures.minecraft.net/texture/abc", + "ftp://textures.minecraft.net/texture/abc", + "file:///etc/passwd", + "http://192.168.0.1/skin.png", + "not a url", + "", + ] { + assert!(!is_allowed_texture_url(bad), "{bad}"); + } + } + + #[test] + fn rejects_textures_property_with_foreign_url() { + use base64::Engine; + + let payload = r#"{"textures":{"SKIN":{"url":"http://192.168.0.1/skin.png"}}}"#; + let value = base64::engine::general_purpose::STANDARD.encode(payload); + assert!(skin_url_from_texture_property(&value).is_err()); + } + fn set_px(img: &mut [u8], x: u32, y: u32, rgba: [u8; 4]) { img[skin_px(x, y)..skin_px(x, y) + 4].copy_from_slice(&rgba); } From d71ac999d660598fd2fee2b1633ead68f3944cb0 Mon Sep 17 00:00:00 2001 From: Purdze <29306641+Purdze@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:10:33 +0100 Subject: [PATCH 2/4] reject the whole textures payload on any foreign url --- pomme-client/src/renderer/mod.rs | 82 ++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 36 deletions(-) diff --git a/pomme-client/src/renderer/mod.rs b/pomme-client/src/renderer/mod.rs index 8b2d3a61f..891ad11da 100644 --- a/pomme-client/src/renderer/mod.rs +++ b/pomme-client/src/renderer/mod.rs @@ -2085,16 +2085,11 @@ pub(crate) async fn fetch_skin_texture_from_profile_property( fn skin_url_from_texture_property(value: &str) -> Result<(String, bool), String> { #[derive(serde::Deserialize)] struct TexturesPayload { - textures: Textures, + textures: std::collections::HashMap, } #[derive(serde::Deserialize)] - struct Textures { - #[serde(rename = "SKIN")] - skin: Option, - } - #[derive(serde::Deserialize)] - struct SkinTexture { - url: String, + struct Texture { + url: Option, metadata: Option, } #[derive(serde::Deserialize)] @@ -2107,34 +2102,42 @@ fn skin_url_from_texture_property(value: &str) -> Result<(String, bool), String> .decode(value) .or_else(|_| base64::engine::general_purpose::STANDARD_NO_PAD.decode(value)) .map_err(error_chain)?; - let payload: TexturesPayload = serde_json::from_slice(&decoded).map_err(error_chain)?; + let mut payload: TexturesPayload = serde_json::from_slice(&decoded).map_err(error_chain)?; + // authlib `unpackTextures`: one bad url empties the whole payload. + // TODO: vanilla also shows the default skin for other players whose + // textures property isn't SIGNED (`PlayerInfo.createSkinLookup`); pomme + // doesn't verify the signature yet. + if let Some(bad) = payload + .textures + .values() + .find(|t| !t.url.as_deref().is_some_and(is_allowed_texture_url)) + { + return Err(format!("texture url not allowed: {:?}", bad.url)); + } let skin = payload .textures - .skin + .remove("SKIN") .ok_or_else(|| "No skin texture".to_string())?; - if !is_allowed_texture_url(&skin.url) { - return Err(format!("texture url not allowed: {}", skin.url)); - } let slim = skin.metadata.as_ref().and_then(|m| m.model.as_deref()) == Some("slim"); - Ok((skin.url, slim)) + Ok((skin.url.unwrap_or_default(), slim)) } -/// authlib's `TextureUrlChecker.isAllowedTextureDomain`: http(s) on exactly -/// `textures.minecraft.net`. Scheme and host are checked on the raw text since -/// `Url` lowercases both and authlib is case-sensitive. +/// authlib's `TextureUrlChecker.isAllowedTextureDomain`: a `java.net.URI` +/// (`parse_untrusted_url`'s rules) with a case-sensitive http(s) scheme and a +/// host of exactly `textures.minecraft.net`. fn is_allowed_texture_url(url: &str) -> bool { const HOST: &str = "textures.minecraft.net"; - let Ok(parsed) = reqwest::Url::parse(url) else { + if crate::chat_component::parse_untrusted_url(url.to_owned()).is_err() { return false; - }; + } let Some((scheme, rest)) = url.split_once("://") else { return false; }; let authority = rest.split(['/', '?', '#']).next().unwrap_or_default(); let host_port = authority.rsplit_once('@').map_or(authority, |(_, h)| h); - let host = host_port.split_once(':').map_or(host_port, |(h, _)| h); - matches!(scheme, "http" | "https") && host == HOST && parsed.host_str() == Some(HOST) + let (host, port) = host_port.split_once(':').unwrap_or((host_port, "")); + matches!(scheme, "http" | "https") && host == HOST && port.bytes().all(|b| b.is_ascii_digit()) } /// Error message including the source chain (`reqwest` hides the detail there). @@ -2325,11 +2328,9 @@ mod tests { #[test] fn decodes_skin_url_from_textures_property() { - use base64::Engine; - let payload = r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/testskin"}}}"#; - let value = base64::engine::general_purpose::STANDARD.encode(payload); + let value = textures_property(payload); assert_eq!( skin_url_from_texture_property(&value).unwrap(), @@ -2342,11 +2343,9 @@ mod tests { #[test] fn decodes_unpadded_skin_url_from_textures_property() { - use base64::Engine; - let payload = r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/testskin"}}}"#; - let value = base64::engine::general_purpose::STANDARD.encode(payload); + let value = textures_property(payload); let value = value.trim_end_matches('='); assert_eq!( @@ -2360,10 +2359,8 @@ mod tests { #[test] fn decodes_slim_model_from_textures_property() { - use base64::Engine; - let payload = r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/testskin","metadata":{"model":"slim"}}}}"#; - let value = base64::engine::general_purpose::STANDARD.encode(payload); + let value = textures_property(payload); assert_eq!( skin_url_from_texture_property(&value).unwrap(), @@ -2374,6 +2371,12 @@ mod tests { ); } + /// A profile's `textures` property value: the payload, base64-encoded. + fn textures_property(payload: &str) -> String { + use base64::Engine; + base64::engine::general_purpose::STANDARD.encode(payload) + } + #[test] fn texture_urls_follow_authlib_allow_list() { for ok in [ @@ -2394,6 +2397,10 @@ mod tests { "ftp://textures.minecraft.net/texture/abc", "file:///etc/passwd", "http://192.168.0.1/skin.png", + "https://textures.minecraft.net/texture/a b", + "https://textures.minecraft.net/texture/a|b", + r"https://textures.minecraft.net\@evil.com/abc", + "https://textures.minecraft.net:abc/texture/abc", "not a url", "", ] { @@ -2402,12 +2409,15 @@ mod tests { } #[test] - fn rejects_textures_property_with_foreign_url() { - use base64::Engine; - - let payload = r#"{"textures":{"SKIN":{"url":"http://192.168.0.1/skin.png"}}}"#; - let value = base64::engine::general_purpose::STANDARD.encode(payload); - assert!(skin_url_from_texture_property(&value).is_err()); + fn any_foreign_texture_url_rejects_the_whole_payload() { + for payload in [ + r#"{"textures":{"SKIN":{"url":"http://192.168.0.1/skin.png"}}}"#, + r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/a"},"CAPE":{"url":"http://192.168.0.1/cape.png"}}}"#, + r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/a"},"ELYTRA":{}}}"#, + ] { + let value = textures_property(payload); + assert!(skin_url_from_texture_property(&value).is_err(), "{payload}"); + } } fn set_px(img: &mut [u8], x: u32, y: u32, rgba: [u8; 4]) { From 0f44d03e210655133aaa69186c8f2b9086b5c564 Mon Sep 17 00:00:00 2001 From: Purdze <29306641+Purdze@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:46:16 +0100 Subject: [PATCH 3/4] match authlib port and duplicate-key handling --- pomme-client/src/renderer/mod.rs | 76 +++++++++++++++++++++++++------- 1 file changed, 59 insertions(+), 17 deletions(-) diff --git a/pomme-client/src/renderer/mod.rs b/pomme-client/src/renderer/mod.rs index 891ad11da..21df0e734 100644 --- a/pomme-client/src/renderer/mod.rs +++ b/pomme-client/src/renderer/mod.rs @@ -2062,6 +2062,7 @@ pub(crate) async fn fetch_skin_texture(uuid: &str) -> Result { .properties .iter() .find(|p| p.name.as_deref() == Some("textures")) + // TODO: authlib reads only the `textures` property; drop this fallback. .or_else(|| profile.properties.first()) .ok_or("No properties")? .value; @@ -2085,8 +2086,9 @@ pub(crate) async fn fetch_skin_texture_from_profile_property( fn skin_url_from_texture_property(value: &str) -> Result<(String, bool), String> { #[derive(serde::Deserialize)] struct TexturesPayload { - textures: std::collections::HashMap, + textures: Textures, } + struct Textures(Vec<(String, Texture)>); #[derive(serde::Deserialize)] struct Texture { url: Option, @@ -2097,30 +2099,63 @@ fn skin_url_from_texture_property(value: &str) -> Result<(String, bool), String> model: Option, } + /// Gson's map adapter fails on a repeated key. Keys are authlib's texture + /// `Type`, so unknown names all collide as `null`. + impl<'de> serde::Deserialize<'de> for Textures { + fn deserialize>(deserializer: D) -> Result { + struct Visitor; + impl<'de> serde::de::Visitor<'de> for Visitor { + type Value = Textures; + + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("a textures map") + } + + fn visit_map>( + self, + mut map: A, + ) -> Result { + fn slot(key: &str) -> Option<&str> { + matches!(key, "SKIN" | "CAPE" | "ELYTRA").then_some(key) + } + let mut entries: Vec<(String, Texture)> = Vec::new(); + while let Some((key, texture)) = map.next_entry::()? { + if entries.iter().any(|(k, _)| slot(k) == slot(&key)) { + return Err(serde::de::Error::custom(format!("duplicate key: {key}"))); + } + entries.push((key, texture)); + } + Ok(Textures(entries)) + } + } + deserializer.deserialize_map(Visitor) + } + } + use base64::Engine; let decoded = base64::engine::general_purpose::STANDARD .decode(value) .or_else(|_| base64::engine::general_purpose::STANDARD_NO_PAD.decode(value)) .map_err(error_chain)?; - let mut payload: TexturesPayload = serde_json::from_slice(&decoded).map_err(error_chain)?; + let payload: TexturesPayload = serde_json::from_slice(&decoded).map_err(error_chain)?; // authlib `unpackTextures`: one bad url empties the whole payload. // TODO: vanilla also shows the default skin for other players whose // textures property isn't SIGNED (`PlayerInfo.createSkinLookup`); pomme // doesn't verify the signature yet. - if let Some(bad) = payload - .textures - .values() - .find(|t| !t.url.as_deref().is_some_and(is_allowed_texture_url)) - { - return Err(format!("texture url not allowed: {:?}", bad.url)); - } - let skin = payload - .textures - .remove("SKIN") - .ok_or_else(|| "No skin texture".to_string())?; - let slim = skin.metadata.as_ref().and_then(|m| m.model.as_deref()) == Some("slim"); - Ok((skin.url.unwrap_or_default(), slim)) + let mut skin = None; + for (key, texture) in payload.textures.0 { + match texture.url { + Some(url) if is_allowed_texture_url(&url) => { + if key == "SKIN" { + let slim = texture.metadata.and_then(|m| m.model).as_deref() == Some("slim"); + skin = Some((url, slim)); + } + } + url => return Err(format!("texture url not allowed: {url:?}")), + } + } + skin.ok_or_else(|| "No skin texture".to_string()) } /// authlib's `TextureUrlChecker.isAllowedTextureDomain`: a `java.net.URI` @@ -2137,7 +2172,11 @@ fn is_allowed_texture_url(url: &str) -> bool { let authority = rest.split(['/', '?', '#']).next().unwrap_or_default(); let host_port = authority.rsplit_once('@').map_or(authority, |(_, h)| h); let (host, port) = host_port.split_once(':').unwrap_or((host_port, "")); - matches!(scheme, "http" | "https") && host == HOST && port.bytes().all(|b| b.is_ascii_digit()) + // `URI` scans the port as digits, then `Integer.parseInt`; an overflow + // leaves no host. + let port_ok = port.is_empty() + || (port.bytes().all(|b| b.is_ascii_digit()) && port.parse::().is_ok()); + matches!(scheme, "http" | "https") && host == HOST && port_ok } /// Error message including the source chain (`reqwest` hides the detail there). @@ -2401,6 +2440,7 @@ mod tests { "https://textures.minecraft.net/texture/a|b", r"https://textures.minecraft.net\@evil.com/abc", "https://textures.minecraft.net:abc/texture/abc", + "https://textures.minecraft.net:99999999999/texture/abc", "not a url", "", ] { @@ -2409,11 +2449,13 @@ mod tests { } #[test] - fn any_foreign_texture_url_rejects_the_whole_payload() { + fn foreign_urls_and_duplicate_keys_reject_the_whole_payload() { for payload in [ r#"{"textures":{"SKIN":{"url":"http://192.168.0.1/skin.png"}}}"#, r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/a"},"CAPE":{"url":"http://192.168.0.1/cape.png"}}}"#, r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/a"},"ELYTRA":{}}}"#, + r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/a"},"SKIN":{"url":"https://textures.minecraft.net/texture/b"}}}"#, + r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/a"},"FOO":{"url":"https://textures.minecraft.net/texture/b"},"BAR":{"url":"https://textures.minecraft.net/texture/c"}}}"#, ] { let value = textures_property(payload); assert!(skin_url_from_texture_property(&value).is_err(), "{payload}"); From 9122ec1ff4e75e045a149d7615005645e4afad9c Mon Sep 17 00:00:00 2001 From: Purdze <29306641+Purdze@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:52:39 +0100 Subject: [PATCH 4/4] cleanup --- pomme-client/src/renderer/mod.rs | 58 ++++++++++---------------------- 1 file changed, 18 insertions(+), 40 deletions(-) diff --git a/pomme-client/src/renderer/mod.rs b/pomme-client/src/renderer/mod.rs index e60ccdef0..aee1ac9ee 100644 --- a/pomme-client/src/renderer/mod.rs +++ b/pomme-client/src/renderer/mod.rs @@ -2452,50 +2452,28 @@ mod tests { #[test] fn decodes_skin_url_from_textures_property() { - let payload = - r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/testskin"}}}"#; - let value = textures_property(payload); - - assert_eq!( - skin_url_from_texture_property(&value).unwrap(), - ( - "https://textures.minecraft.net/texture/testskin".into(), - false - ) - ); - } - - #[test] - fn decodes_unpadded_skin_url_from_textures_property() { - let payload = - r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/testskin"}}}"#; - let value = textures_property(payload); - let value = value.trim_end_matches('='); - - assert_eq!( - skin_url_from_texture_property(value).unwrap(), - ( - "https://textures.minecraft.net/texture/testskin".into(), - false - ) + let classic = textures_property( + r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/testskin"}}}"#, ); - } - - #[test] - fn decodes_slim_model_from_textures_property() { - let payload = r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/testskin","metadata":{"model":"slim"}}}}"#; - let value = textures_property(payload); - - assert_eq!( - skin_url_from_texture_property(&value).unwrap(), - ( - "https://textures.minecraft.net/texture/testskin".into(), - true - ) + let slim = textures_property( + r#"{"textures":{"SKIN":{"url":"https://textures.minecraft.net/texture/testskin","metadata":{"model":"slim"}}}}"#, ); + for (value, is_slim) in [ + (classic.as_str(), false), + (classic.trim_end_matches('='), false), + (slim.as_str(), true), + ] { + assert_eq!( + skin_url_from_texture_property(value).unwrap(), + ( + "https://textures.minecraft.net/texture/testskin".to_owned(), + is_slim + ), + "{value}" + ); + } } - /// A profile's `textures` property value: the payload, base64-encoded. fn textures_property(payload: &str) -> String { use base64::Engine; base64::engine::general_purpose::STANDARD.encode(payload)