Fixing kernel commandline parameter validation

According to linux kernel document, the value of command line parameter can contains spaces (with double quotes) or equals. This change will also reject values that contains quotes in the middle. Signed-off-by: Nguyen Dinh Phi <[email protected]>
rust-vmm · Nov 27, 2024 · 2f1d19a · 2f1d19a
1 parent 4c78198
commit 2f1d19a
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,8 @@
 # Upcoming Release
+## Changed
+- [[#148](https://github.com/rust-vmm/linux-loader/pull/148)] Fixing kernel commandline parameter validation.
+  This change allows parameter values containing spaces in the middle, provided they are enclosed in quotes. However, 
+  strings with quotes in the middle will no longer be accepted as valid parameter values.
 
 # [v0.13.0]
 

diff --git a/src/cmdline/mod.rs b/src/cmdline/mod.rs
@@ -37,6 +37,10 @@ pub enum Error {
     MmioSize,
     /// Operation would have made the command line too large.
     TooLarge,
+    /// Double-quotes can be used to protect spaces in values
+    NoQuoteSpace,
+    /// A double quote that is in the middle of the value
+    InvalidQuote,
 }
 
 impl fmt::Display for Error {
@@ -56,6 +60,11 @@ impl fmt::Display for Error {
                 "0-sized virtio MMIO device passed to the kernel command line builder."
             ),
             Error::TooLarge => write!(f, "Inserting string would make command line too long."),
+            Error::NoQuoteSpace => write!(
+                f,
+                "Value that contains spaces need to be surrounded by quotes"
+            ),
+            Error::InvalidQuote => write!(f, "Double quote can not be in the middle of the value"),
         }
     }
 }
@@ -79,7 +88,27 @@ fn valid_str(s: &str) -> Result<()> {
     }
 }
 
-fn valid_element(s: &str) -> Result<()> {
+fn is_quoted(s: &str) -> bool {
+    if s.len() < 2 {
+        return false;
+    }
+
+    if let Some(first_char) = s.chars().next() {
+        if let Some(last_char) = s.chars().last() {
+            return first_char == '"' && last_char == '"';
+        }
+    }
+    false
+}
+
+fn contains_double_quotes(s: &str) -> bool {
+    if s.len() < 3 {
+        return false;
+    }
+    return s.chars().skip(1).take(s.len() - 2).any(|c| c == '"');
+}
+
+fn valid_key(s: &str) -> Result<()> {
     if !s.chars().all(valid_char) {
         Err(Error::InvalidAscii)
     } else if s.contains(' ') {
@@ -91,6 +120,18 @@ fn valid_element(s: &str) -> Result<()> {
     }
 }
 
+fn valid_value(s: &str) -> Result<()> {
+    if !s.chars().all(valid_char) {
+        Err(Error::InvalidAscii)
+    } else if contains_double_quotes(s) {
+        Err(Error::InvalidQuote)
+    } else if s.contains(' ') && !is_quoted(s) {
+        Err(Error::NoQuoteSpace)
+    } else {
+        Ok(())
+    }
+}
+
 /// A builder for a kernel command line string that validates the string as it's being built.
 ///
 /// # Examples
@@ -155,8 +196,8 @@ impl Cmdline {
         let k = key.as_ref();
         let v = val.as_ref();
 
-        valid_element(k)?;
-        valid_element(v)?;
+        valid_key(k)?;
+        valid_value(v)?;
 
         let kv_str = format!("{}={}", k, v);
 
@@ -186,7 +227,7 @@ impl Cmdline {
     pub fn insert_multiple<T: AsRef<str>>(&mut self, key: T, vals: &[T]) -> Result<()> {
         let k = key.as_ref();
 
-        valid_element(k)?;
+        valid_key(k)?;
         if vals.is_empty() {
             return Err(Error::MissingVal(k.to_string()));
         }
@@ -196,7 +237,7 @@ impl Cmdline {
             k,
             vals.iter()
                 .map(|v| -> Result<&str> {
-                    valid_element(v.as_ref())?;
+                    valid_value(v.as_ref())?;
                     Ok(v.as_ref())
                 })
                 .collect::<Result<Vec<&str>>>()?
@@ -567,20 +608,29 @@ mod tests {
     fn test_insert_space() {
         let mut cl = Cmdline::new(100).unwrap();
         assert_eq!(cl.insert("a ", "b"), Err(Error::HasSpace));
-        assert_eq!(cl.insert("a", "b "), Err(Error::HasSpace));
+        assert_eq!(cl.insert("a", "b "), Err(Error::NoQuoteSpace));
         assert_eq!(cl.insert("a ", "b "), Err(Error::HasSpace));
         assert_eq!(cl.insert(" a", "b"), Err(Error::HasSpace));
+        assert_eq!(cl.insert("a", "hello \"world"), Err(Error::InvalidQuote));
+        assert_eq!(
+            cl.insert("a", "\"foor bar\" \"foor bar\""),
+            Err(Error::InvalidQuote)
+        );
         assert_eq!(cl.as_cstring().unwrap().as_bytes_with_nul(), b"\0");
+        assert!(cl.insert("a", "\"b b\"").is_ok());
+        assert!(cl.insert("c", "\" d\"").is_ok());
+        assert_eq!(
+            cl.as_cstring().unwrap().as_bytes_with_nul(),
+            b"a=\"b b\" c=\" d\"\0"
+        );
     }
 
     #[test]
     fn test_insert_equals() {
         let mut cl = Cmdline::new(100).unwrap();
         assert_eq!(cl.insert("a=", "b"), Err(Error::HasEquals));
-        assert_eq!(cl.insert("a", "b="), Err(Error::HasEquals));
         assert_eq!(cl.insert("a=", "b "), Err(Error::HasEquals));
         assert_eq!(cl.insert("=a", "b"), Err(Error::HasEquals));
-        assert_eq!(cl.insert("a", "=b"), Err(Error::HasEquals));
         assert_eq!(cl.as_cstring().unwrap().as_bytes_with_nul(), b"\0");
     }
 
@@ -702,7 +752,10 @@ mod tests {
             cl.insert_multiple("foo", &no_vals),
             Err(Error::MissingVal("foo".to_string()))
         );
-        assert_eq!(cl.insert_multiple("foo", &["bar "]), Err(Error::HasSpace));
+        assert_eq!(
+            cl.insert_multiple("foo", &["bar "]),
+            Err(Error::NoQuoteSpace)
+        );
         assert_eq!(
             cl.insert_multiple("foo", &["bar", "baz"]),
             Err(Error::TooLarge)