[Bug] Identifier::Deserialize bypasses is_valid — BCS creates invalid Identifiers

[Esorat]

Bug

Identifier uses #[derive(Deserialize)] which creates instances from raw bytes without calling is_valid. This allows BCS deserialization to produce Identifiers containing \n, \0, or any other invalid characters — violating the type invariant that is_valid should always return true.

To reproduce

use move_core_types::{identifier::Identifier, language_storage::TypeTag};

// BCS bytes containing 0x0A (newline) inside identifier data
let bcs_bytes: &[u8] = &[
    7, 255, 158, 1, 33, 0, 255, 255, 255, 0, 255, 255, 255, 255,
    10, 10, 10, 10, 10, 10, 10, 10, 83, 10, 10, 10, 10, 10, 10, 10, 10, 10,
    103, 10, 10, 10, 10, 10, 10, 10, 10, 10, 0, 0, 0, 0, 0, 0, 0, 0,
];

let tag: TypeTag = bcs::from_bytes(bcs_bytes).unwrap();

if let TypeTag::Struct(s) = &tag {
    assert!(!Identifier::is_valid(s.module.as_str())); // is_valid=false but deserialized
    assert!(!Identifier::is_valid(s.name.as_str()));
}

Result:

module: "\n\n\n\nS\n\n\n\n\n"  is_valid=false
name:   "\n\n\ng\n\n\n\n\n\n"  is_valid=false

Impact: Display-to-Parse divergence

let display = tag.to_string();
let parsed = TypeTag::from_str(&display).unwrap();
assert_ne!(tag, parsed); // Parser strips whitespace, produces different TypeTag

Root cause

identifier.rs — derived Deserialize reads raw bytes directly, no is_valid check:

#[derive(Deserialize)]
pub struct Identifier(Box<str>);

Suggested fix

Custom Deserialize with validation:

impl<'de> Deserialize<'de> for Identifier {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where D: Deserializer<'de> {
        let s = Box::<str>::deserialize(deserializer)?;
        if !Self::is_valid(&s) {
            return Err(de::Error::custom("invalid identifier"));
        }
        Ok(Identifier(s))
    }
}

[PrazwalR]

Opened a fix in #20056 — custom Deserialize validating via Identifier::new, wire-format compatible, with regression tests.