diff --git a/pomme-client/src/renderer/mod.rs b/pomme-client/src/renderer/mod.rs index 7ec4c9b3c..aee1ac9ee 100644 --- a/pomme-client/src/renderer/mod.rs +++ b/pomme-client/src/renderer/mod.rs @@ -2147,6 +2147,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; @@ -2172,14 +2173,10 @@ fn skin_url_from_texture_property(value: &str) -> Result<(String, bool), String> struct TexturesPayload { textures: Textures, } + struct Textures(Vec<(String, Texture)>); #[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)] @@ -2187,6 +2184,39 @@ 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) @@ -2194,14 +2224,44 @@ 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 - .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()) + // 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. + 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` +/// (`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"; + 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, port) = host_port.split_once(':').unwrap_or((host_port, "")); + // `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). @@ -2392,53 +2452,77 @@ 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); - - 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"}}}"#, + ); + 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}" + ); + } } - #[test] - fn decodes_unpadded_skin_url_from_textures_property() { + fn textures_property(payload: &str) -> String { 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 = value.trim_end_matches('='); - - assert_eq!( - skin_url_from_texture_property(value).unwrap(), - ( - "https://textures.minecraft.net/texture/testskin".into(), - false - ) - ); + base64::engine::general_purpose::STANDARD.encode(payload) } #[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); + 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", + "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", + "https://textures.minecraft.net:99999999999/texture/abc", + "not a url", + "", + ] { + assert!(!is_allowed_texture_url(bad), "{bad}"); + } + } - assert_eq!( - skin_url_from_texture_property(&value).unwrap(), - ( - "https://textures.minecraft.net/texture/testskin".into(), - true - ) - ); + #[test] + 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}"); + } } fn set_px(img: &mut [u8], x: u32, y: u32, rgba: [u8; 4]) {