Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing kernel commandline parameter validation #148

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 41 additions & 9 deletions src/cmdline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ 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,
}

impl fmt::Display for Error {
Expand All @@ -56,6 +58,10 @@ 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"
),
}
}
}
Expand All @@ -79,7 +85,16 @@ fn valid_str(s: &str) -> Result<()> {
}
}

fn valid_element(s: &str) -> Result<()> {
fn is_quoted(s: &str) -> bool {
if let Some(first_char) = s.chars().next() {
if let Some(last_char) = s.chars().last() {
return first_char == '"' && last_char == '"';
}
}
false
}

fn valid_key(s: &str) -> Result<()> {
if !s.chars().all(valid_char) {
Err(Error::InvalidAscii)
} else if s.contains(' ') {
Expand All @@ -91,6 +106,16 @@ fn valid_element(s: &str) -> Result<()> {
}
}

fn valid_value(s: &str) -> Result<()> {
if !s.chars().all(valid_char) {
Err(Error::InvalidAscii)
} 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
Expand Down Expand Up @@ -155,8 +180,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);

Expand Down Expand Up @@ -186,7 +211,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()));
}
Expand All @@ -196,7 +221,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>>>()?
Expand Down Expand Up @@ -567,20 +592,24 @@ 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.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");
}

Expand Down Expand Up @@ -702,7 +731,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)
Expand Down