Repository: duesee/imap-codec (language: rust)

Issue:
misuse: Deserialization into incorrect types
Currently, types created through `deserialize` are not validated. Thus, it is possible to create incorrect values, e.g.:
```rust
#!/usr/bin/env rust-script
//! Try to deserialize invalid input into a `core::Text` type
//!
//! ```cargo
//! [dependencies]
//! imap-types = { git = "https://github.com/duesee/imap-codec.git", features = ["serde"] }
//! serde_json = "1.0"
//! ```

use imap_types::core::Text;

fn main() {
    let valid_input = r#""Hello, world!""#;
    let invalid_input = r#""Hello,\rworld!""#;

    let text = serde_json::from_str::<Text>(valid_input)
        .expect("valid input should deserialize successfully");
    assert_eq!(text.as_ref(), "Hello, world!");

    let err = serde_json::from_str::<Text>(invalid_input)
        .expect_err("invalid input should not deserialize successfully");
    assert_eq!(
        err.to_string(),
        r"Validation failed: Invalid byte b'\x0d' at index 6"
    );

    println!("Success.")
}
```

```bash
$ rust-script deserialize_invalid_text.ers    
thread 'main' panicked at ...\deserialize_invalid_text.ers:21:10:
invalid input should not deserialize successfully: Text("Hello,\rworld!")
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

At least for `core::Text`, it is possible to fix the issue using [`#[serde(try_from = "String")]`](https://serde.rs/container-attrs.html#try_from).

I will try to provide a corresponding PR in the next days.


Discussion / hints:
Thank you very much, Henning! As a short explainer: The `serde` support happened "by accident" because we needed it quickly for [another project](https://github.com/Email-Analysis-Toolkit/fake-mail-server) :-) I never saw it as part of the misuse resistancy contract. But: This is inconsequent (and not documented).

Now, with the Python bindings, we have a good reason to finally rectify that. Consequently, we should extend the fuzz targets to capture `serde`s (de)serialization, too. (Let's think about how to do it later.)

Good news: I believe we only need a few tweaks here and there. Deserialization is guaranteed to be misuse resistant for tightly modelled types, i.e., types that enforce all invariants by definition.

This is not true for "string types", such as `Atom`, etc. Thus, I expect most of the validated deserialization will need to happen in the `core` module. It should really only be a handful of types.

(Maybe `CommandContinuationRequest` will be tricky. But we should get quite far.)

Fix patch:
diff --git a/imap-types/src/core.rs b/imap-types/src/core.rs
index e6f0a2b6..eaa70381 100644
--- a/imap-types/src/core.rs
+++ b/imap-types/src/core.rs
@@ -98,6 +98,7 @@ use crate::extensions::binary::Literal8;
 /// ```
 #[cfg_attr(feature = "bounded-static", derive(ToStatic))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "serde", serde(try_from = "String"))]
 #[derive(Clone, PartialEq, Eq, Ord, PartialOrd, Hash)]
 pub struct Atom<'a>(pub(crate) Cow<'a, str>);
 
@@ -250,6 +251,7 @@ impl<'a> Display for Atom<'a> {
 /// ```
 #[cfg_attr(feature = "bounded-static", derive(ToStatic))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "serde", serde(try_from = "String"))]
 #[derive(Clone, PartialEq, Eq, Hash)]
 pub struct AtomExt<'a>(pub(crate) Cow<'a, str>);
 
@@ -502,6 +504,10 @@ impl<'a> AsRef<[u8]> for IString<'a> {
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[derive(Clone, PartialEq, Eq, Hash)]
 pub struct Literal<'a> {
+    #[cfg_attr(
+        feature = "serde",
+        serde(deserialize_with = "deserialize_literal_data")
+    )]
     pub(crate) data: Cow<'a, [u8]>,
     /// Specifies whether this is a synchronizing or non-synchronizing literal.
     ///
@@ -513,6 +519,16 @@ pub struct Literal<'a> {
     pub(crate) mode: LiteralMode,
 }
 
+#[cfg(feature = "serde")]
+fn deserialize_literal_data<'de, 'a, D>(deserializer: D) -> Result<Cow<'a, [u8]>, D::Error>
+where
+    D: serde::de::Deserializer<'de>,
+{
+    let data = Vec::deserialize(deserializer)?;
+    Literal::validate(&data).map_err(serde::de::Error::custom)?;
+    Ok(Cow::Owned(data))
+}
+
 // We want a more readable `Debug` implementation.
 impl<'a> Debug for Literal<'a> {
     fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
@@ -722,6 +738,7 @@ pub enum LiteralMode {
 /// ```
 #[cfg_attr(feature = "bounded-static", derive(ToStatic))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "serde", serde(try_from = "String"))]
 #[derive(Clone, PartialEq, Eq, Hash)]
 pub struct Quoted<'a>(pub(crate) Cow<'a, str>);
 
@@ -1012,6 +1029,7 @@ impl<'a> AsRef<[u8]> for AString<'a> {
 /// ```
 #[cfg_attr(feature = "bounded-static", derive(ToStatic))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "serde", serde(try_from = "String"))]
 #[derive(PartialEq, Eq, Hash, Clone)]
 pub struct Tag<'a>(pub(crate) Cow<'a, str>);
 
@@ -1141,6 +1159,7 @@ impl<'a> AsRef<str> for Tag<'a> {
 /// ```
 #[cfg_attr(feature = "bounded-static", derive(ToStatic))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "serde", serde(try_from = "String"))]
 #[derive(PartialEq, Eq, Hash, Clone)]
 pub struct Text<'a>(pub(crate) Cow<'a, str>);
 
@@ -1270,6 +1289,7 @@ impl<'a> AsRef<str> for Text<'a> {
 /// ```
 #[cfg_attr(feature = "bounded-static", derive(ToStatic))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "serde", serde(try_from = "char"))]
 #[derive(Copy, Debug, PartialEq, Eq, Hash, Clone)]
 pub struct QuotedChar(char);
 
@@ -1327,13 +1347,13 @@ impl TryFrom<char> for QuotedChar {
 /// looking at the `search` rule. (See [#266](https://github.com/duesee/imap-codec/issues/266).)
 ///
 /// ```abnf
-/// search          = "SEARCH" [SP "CHARSET" SP astring] 1*(SP search-key)
-/// ;                                           ^^^^^^^
-/// ;                                           |
-/// ;                                           `Charset`
-//                     ; CHARSET argument to MUST be registered with IANA
+/// search = "SEARCH" [SP "CHARSET" SP astring] 1*(SP search-key)
+///            ;                       ^^^^^^^
+///            ;                       |
+///            ;                       `Charset`
+///            ; CHARSET argument to MUST be registered with IANA
 /// ```
-/// 
+///
 /// So, it seems that it should be an `AString`. However the IMAP standard also points to ...
 /// ```abnf
 /// mime-charset       = 1*mime-charset-chars
@@ -1445,6 +1465,7 @@ pub enum NString8<'a> {
 /// * `Vec<T, 1>` must not be used. Please use the alias [`Vec1<T>`] instead.
 #[cfg_attr(feature = "bounded-static", derive(ToStatic))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "serde", serde(try_from = "Vec<T>"))]
 #[derive(Clone, PartialEq, Eq, Hash)]
 pub struct VecN<T, const N: usize>(pub(crate) Vec<T>);
 
@@ -1957,4 +1978,151 @@ mod tests {
         assert!(VecN::<u8, 2>::try_from(vec![1]).is_err());
         assert!(VecN::<u8, 2>::try_from(vec![1, 2]).is_ok());
     }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_text() {
+        let valid_input = r#""Hello, world!""#;
+        let invalid_input = r#""Hello,\rworld!""#;
+
+        let text = serde_json::from_str::<Text>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(text, Text(Cow::Borrowed("Hello, world!")));
+
+        let err = serde_json::from_str::<Text>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(
+            err.to_string(),
+            r"Validation failed: Invalid byte b'\x0d' at index 6"
+        );
+    }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_atom() {
+        let valid_input = r#""OneWord""#;
+        let invalid_input = r#""Two Words""#;
+
+        let atom = serde_json::from_str::<Atom>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(atom, Atom(Cow::Borrowed("OneWord")));
+
+        let err = serde_json::from_str::<Atom>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(
+            err.to_string(),
+            r"Validation failed: Invalid byte b'\x20' at index 3"
+        );
+    }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_extended_atom() {
+        let valid_input = r#""OneWord""#;
+        let invalid_input = r#""Two Words""#;
+
+        let atom_ext = serde_json::from_str::<AtomExt>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(atom_ext, AtomExt(Cow::Borrowed("OneWord")));
+
+        let err = serde_json::from_str::<AtomExt>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(
+            err.to_string(),
+            r"Validation failed: Invalid byte b'\x20' at index 3"
+        );
+    }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_literal() {
+        let valid_input = r#"{ "data": [ 1, 2, 3 ], "mode": "Sync" }"#;
+        let invalid_input = r#"{ "data": [ 0, 1, 2, 3 ], "mode": "Sync" }"#;
+
+        let literal = serde_json::from_str::<Literal>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(
+            literal,
+            Literal {
+                data: Cow::Borrowed(b"\x01\x02\x03"),
+                mode: LiteralMode::Sync
+            }
+        );
+
+        let err = serde_json::from_str::<Literal>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(
+            err.to_string(),
+            r"Validation failed: Invalid byte b'\x00' at index 0 at line 1 column 24"
+        );
+    }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_quoted() {
+        let valid_input = r#""Hello, world!""#;
+        let invalid_input = r#""Hello,\rworld!""#;
+
+        let quoted = serde_json::from_str::<Quoted>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(quoted, Quoted(Cow::Borrowed("Hello, world!")));
+
+        let err = serde_json::from_str::<Quoted>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(
+            err.to_string(),
+            r"Validation failed: Invalid byte b'\x0d' at index 6"
+        );
+    }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_tag() {
+        let valid_input = r#""A0001""#;
+        let invalid_input = r#""A+0001""#;
+
+        let tag = serde_json::from_str::<Tag>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(tag, Tag(Cow::Borrowed("A0001")));
+
+        let err = serde_json::from_str::<Tag>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(
+            err.to_string(),
+            r"Validation failed: Invalid byte b'\x2b' at index 1"
+        );
+    }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_quoted_char() {
+        let valid_input = r#""A""#;
+        let invalid_input = r#""\r""#;
+
+        let quoted_char = serde_json::from_str::<QuotedChar>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(quoted_char, QuotedChar('A'));
+
+        let err = serde_json::from_str::<QuotedChar>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(err.to_string(), r"Validation failed: Invalid value");
+    }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_vec_n() {
+        let valid_input = r#"[1, 2, 3]"#;
+        let invalid_input = r#"[1, 2]"#;
+
+        let vec_n = serde_json::from_str::<VecN<u8, 3>>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(vec_n, VecN(vec![1, 2, 3]));
+
+        let err = serde_json::from_str::<VecN<u8, 3>>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(
+            err.to_string(),
+            r"Validation failed: Must have at least 3 elements"
+        );
+    }
 }
diff --git a/imap-types/src/datetime.rs b/imap-types/src/datetime.rs
index 85bb0e20..8f65adf9 100644
--- a/imap-types/src/datetime.rs
+++ b/imap-types/src/datetime.rs
@@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize};
 use crate::datetime::error::{DateTimeError, NaiveDateError};
 
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "serde", serde(try_from = "chrono::DateTime<FixedOffset>"))]
 #[derive(Clone, Eq, PartialEq, Hash)]
 pub struct DateTime(chrono::DateTime<FixedOffset>);
 
@@ -96,6 +97,7 @@ impl ToBoundedStatic for DateTime {
 }
 
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "serde", serde(try_from = "chrono::NaiveDate"))]
 #[derive(Clone, Eq, PartialEq, Hash)]
 pub struct NaiveDate(chrono::NaiveDate);
 
@@ -268,4 +270,45 @@ mod tests {
             assert_eq!(expected, got.unwrap_err());
         }
     }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_date_time() {
+        let valid_input = r#""2015-05-15T11:22:33+01:00""#;
+        let invalid_input = r#""+12015-05-15T11:22:33+01:00""#;
+
+        let date_time = serde_json::from_str::<DateTime>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(
+            date_time,
+            DateTime(
+                chrono::FixedOffset::east_opt(3600)
+                    .unwrap()
+                    .with_ymd_and_hms(2015, 5, 15, 11, 22, 33)
+                    .unwrap(),
+            ),
+        );
+
+        let err = serde_json::from_str::<DateTime>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(err.to_string(), r"expected `0 <= year <= 9999`, got 12015");
+    }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_naive_date() {
+        let valid_input = r#""2015-05-15""#;
+        let invalid_input = r#""+12015-05-15""#;
+
+        let naive_date = serde_json::from_str::<NaiveDate>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(
+            naive_date,
+            NaiveDate(chrono::NaiveDate::from_ymd_opt(2015, 5, 15).unwrap()),
+        );
+
+        let err = serde_json::from_str::<NaiveDate>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(err.to_string(), r"expected `0 <= year <= 9999`, got 12015");
+    }
 }
diff --git a/imap-types/src/mailbox.rs b/imap-types/src/mailbox.rs
index 72069306..23804b44 100644
--- a/imap-types/src/mailbox.rs
+++ b/imap-types/src/mailbox.rs
@@ -18,6 +18,7 @@ use crate::{
 
 #[cfg_attr(feature = "bounded-static", derive(ToStatic))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "serde", serde(try_from = "String"))]
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct ListCharString<'a>(pub(crate) Cow<'a, str>);
 
@@ -200,6 +201,7 @@ impl<'a> From<AString<'a>> for Mailbox<'a> {
 
 #[cfg_attr(feature = "bounded-static", derive(ToStatic))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "serde", serde(try_from = "AString<'a>"))]
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct MailboxOther<'a>(pub(crate) AString<'a>);
 
@@ -292,6 +294,8 @@ mod tests {
     use std::borrow::Cow;
 
     use super::*;
+    #[cfg(feature = "serde")]
+    use crate::core::AtomExt;
     use crate::core::{AString, IString, Literal, LiteralMode};
 
     #[test]
@@ -329,4 +333,43 @@ mod tests {
             assert!(Mailbox::try_from(String::from(test)).is_err());
         }
     }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_list_char_string() {
+        let valid_input = r#""OneWord""#;
+        let invalid_input = r#""Two Words""#;
+
+        let list_char_string = serde_json::from_str::<ListCharString>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(list_char_string, ListCharString(Cow::Borrowed("OneWord")));
+
+        let err = serde_json::from_str::<ListCharString>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(
+            err.to_string(),
+            r"Validation failed: Invalid byte b'\x20' at index 3"
+        );
+    }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_mailbox_other() {
+        let valid_input = r#"{ "Atom": "other" }"#;
+        let invalid_input = r#"{ "Atom": "inbox" }"#;
+
+        let mailbox_other = serde_json::from_str::<MailboxOther>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(
+            mailbox_other,
+            MailboxOther(AString::Atom(AtomExt(Cow::Borrowed("other"))))
+        );
+
+        let err = serde_json::from_str::<MailboxOther>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(
+            err.to_string(),
+            r"Reserved: Please use one of the typed variants"
+        );
+    }
 }
diff --git a/imap-types/src/response.rs b/imap-types/src/response.rs
index 950aabcb..bbd60b77 100644
--- a/imap-types/src/response.rs
+++ b/imap-types/src/response.rs
@@ -684,12 +684,36 @@ impl<'a> CommandContinuationRequest<'a> {
 
 #[cfg_attr(feature = "bounded-static", derive(ToStatic))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(
+    feature = "serde",
+    serde(try_from = "CommandContinuationRequestBasicShadow")
+)]
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct CommandContinuationRequestBasic<'a> {
     code: Option<Code<'a>>,
     text: Text<'a>,
 }
 
+/// Use shadow type to support validated deserialization
+/// until `serde` provides built-in support for this case.
+#[cfg(feature = "serde")]
+#[derive(Deserialize, Debug)]
+struct CommandContinuationRequestBasicShadow<'a> {
+    code: Option<Code<'a>>,
+    text: Text<'a>,
+}
+
+#[cfg(feature = "serde")]
+impl<'a> TryFrom<CommandContinuationRequestBasicShadow<'a>>
+    for CommandContinuationRequestBasic<'a>
+{
+    type Error = ContinueError<std::convert::Infallible>;
+
+    fn try_from(value: CommandContinuationRequestBasicShadow<'a>) -> Result<Self, Self::Error> {
+        Self::new(value.code, value.text)
+    }
+}
+
 impl<'a> CommandContinuationRequestBasic<'a> {
     /// Create a basic continuation request.
     ///
@@ -1236,4 +1260,25 @@ mod tests {
             assert!(test.is_err());
         }
     }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_deserialization_command_continuation_request_basic() {
+        let valid_input = r#"{ "text": "Ready for additional command text" }"#;
+        let invalid_input = r#"{ "text": "[Ready for additional command text" }"#;
+
+        let request = serde_json::from_str::<CommandContinuationRequestBasic>(valid_input)
+            .expect("valid input should deserialize successfully");
+        assert_eq!(
+            request,
+            CommandContinuationRequestBasic {
+                code: None,
+                text: Text(Cow::Borrowed("Ready for additional command text"))
+            }
+        );
+
+        let err = serde_json::from_str::<CommandContinuationRequestBasic>(invalid_input)
+            .expect_err("invalid input should not deserialize successfully");
+        assert_eq!(err.to_string(), r"ambiguity detected");
+    }
 }


Test patch: