CVE-2019-16214 (Other, published 2019-09-11T04:15Z)
Description: value: Libra Core before 2019-09-03 has an erroneous regular expression for inline comments, which makes it easier for attackers to interfere with code auditing by using a nonstandard line-break character for a comment. For example, a Move module author can enter the // sequence (which introduces a single-line comment), followed by very brief comment text, the \r character, and code that has security-critical functionality. In many popular environments, this code is displayed on a separate line, and thus a reader may infer that the code is executed. However, the code is NOT executed, because language/compiler/ir_to_bytecode/src/parser.rs allows the comment to continue after the \r character.
Severity: LOW (CVSS v3 base score: 5.7)
Weakness: NVD-CWE-noinfo \u2014 Insufficient Information
There is insufficient information about the issue to classify it; details are unkown or unspecified.
Repository: https://github.com/libra/libra
Files changed:
language/compiler/ir_to_bytecode/Cargo.toml
language/compiler/ir_to_bytecode/src/parser.rs
Commit message: [language] Restrict character classes allowed in IR

This PR restricts the character classes that are allowed in IR files to
either be:
* ASCII printable (space through ~).
* \n for newlines
* \t for tabs
All other characters anywhere in the file will result in an error being
raised before the string is even passed to the parser.


Vulnerable code:
regex = "1.2.1"
use regex::Regex;
// Since lalrpop can't handle comments without a custom lexer, we somewhat hackily remove all the
// comments from the input string before passing it off to lalrpop. We only support single line
// comments for now. Will later on add in other comment types.
fn strip_comments(string: &str) -> String {
    // Remove line comments
    let line_comments = Regex::new(r"(?m)//.*$").unwrap();
    line_comments.replace_all(string, "$1").into_owned()
    let stripped_string = &strip_comments(program_str);
        Err(e) => handle_error(e, program_str),
    let stripped_string = &strip_comments(script_str);
        Err(e) => handle_error(e, script_str),
    let stripped_string = &strip_comments(modules_str);
        Err(e) => handle_error(e, modules_str),
    let stripped_string = &strip_comments(cmd_str);
        Err(e) => handle_error(e, cmd_str),

Fixed code:
/// Determine if a character is an allowed eye-visible (printable) character.
///
/// The only allowed printable characters are the printable ascii characters (SPACE through ~) and
/// tabs. All other characters are invalid and we return false.
pub fn is_permitted_printable_char(c: char) -> bool {
    let x = c as u32;
    let is_above_space = x >= 0x20; // Don't allow meta characters
    let is_below_tilde = x <= 0x7E; // Don't allow DEL meta character
    let is_tab = x == 0x09; // Allow tabs
    (is_above_space && is_below_tilde) || is_tab
}

/// Determine if a character is a permitted newline character.
///
/// The only permitted newline character is \n. All others are invalid.
pub fn is_permitted_newline_char(c: char) -> bool {
    let x = c as u32;
    x == 0x0A
}

/// Determine if a character is permitted character.
///
/// A permitted character is either a permitted printable character, or a permitted
/// newline. Any other characters are disallowed from appearing in the file.
pub fn is_permitted_char(c: char) -> bool {
    is_permitted_printable_char(c) || is_permitted_newline_char(c)
}

fn verify_string(string: &str) -> Result<()> {
    match string.chars().find(|c| !is_permitted_char(*c)) {
        None => Ok(()),
        Some(chr) => bail!(
            "Parser Error: invalid character {} found when reading file.\
             Only ascii printable, tabs (\\t), and \\n line ending characters are permitted.",
            chr
        ),
    }
}

fn strip_comments(source: &str) -> String {
    const SLASH: char = '/';
    const SPACE: char = ' ';

    let mut in_comment = false;
    let mut acc = String::with_capacity(source.len());
    let mut char_iter = source.chars().peekable();

    while let Some(chr) = char_iter.next() {
        let at_newline = is_permitted_newline_char(chr);
        let at_or_after_slash_slash =
            in_comment || (chr == SLASH && char_iter.peek().map(|c| *c == SLASH).unwrap_or(false));
        in_comment = !at_newline && at_or_after_slash_slash;
        acc.push(if in_comment { SPACE } else { chr });
    }

    acc
}

// We restrict strings to only ascii visual characters (0x20 <= c <= 0x7E) or a permitted newline
// character--\n--or a tab--\t.
fn strip_comments_and_verify(string: &str) -> Result<String> {
    verify_string(string)?;
    Ok(strip_comments(string))
    let stripped_string = &strip_comments_and_verify(program_str)?;
        Err(e) => handle_error(e, stripped_string),
    let stripped_string = &strip_comments_and_verify(script_str)?;
        Err(e) => handle_error(e, stripped_string),
    let stripped_string = &strip_comments_and_verify(modules_str)?;
        Err(e) => handle_error(e, stripped_string),
    let stripped_string = &strip_comments_and_verify(cmd_str)?;
        Err(e) => handle_error(e, stripped_string),

#[cfg(test)]
mod tests {
    #[test]
    fn verify_character_whitelist() {
        let mut good_chars = (0x20..=0x7E).collect::<Vec<u8>>();
        good_chars.push(0x0A);
        good_chars.push(0x09);

        let mut bad_chars = (0x0..0x09).collect::<Vec<_>>();
        bad_chars.append(&mut (0x0B..=0x1F).collect::<Vec<_>>());
        bad_chars.push(0x7F);

        // Test to make sure that all the characters that are in the whitelist pass.
        {
            let s = std::str::from_utf8(&good_chars)
                .expect("Failed to construct string containing an invalid character. This shouldn't happen.");
            assert!(super::verify_string(s).is_ok());
        }

        // Test to make sure that we fail for all characters not in the whitelist.
        for bad_char in bad_chars {
            good_chars.push(bad_char);
            let s = std::str::from_utf8(&good_chars)
                .expect("Failed to construct string containing an invalid character. This shouldn't happen.");
            assert!(super::verify_string(s).is_err());
            good_chars.pop();
        }
    }

    #[test]
    fn test_strip_comments() {
        let mut good_chars = (0x20..=0x7E).map(|x: u8| x as char).collect::<String>();
        good_chars.push(0x09 as char);
        good_chars.push(0x0A as char);
        good_chars.insert(0, 0x2F as char);
        good_chars.insert(0, 0x2F as char);

        {
            let x = super::strip_comments(&good_chars);
            assert!(x.chars().all(|x| x == ' ' || x == '\t' || x == '\n'));
        }

        // Remove the \n at the end of the line
        good_chars.pop();

        let bad_chars: Vec<u8> = vec![
            0x0B, // VT
            0x0C, // FF
            0x0D, // CR
            0x0D, 0x0A, // CRLF
            0xC2, 0x85, // NEL
            0xE2, 0x80, 0xA8, // LS
            0xE2, 0x80, 0xA9, // PS
            0x1E, // RS
            0x15, // NL
            0x76, // NEWLINE
        ];

        let bad_chars = std::str::from_utf8(&bad_chars).expect(
            "Failed to construct string containing an invalid character. This shouldn't happen.",
        );
        for bad_char in bad_chars.chars() {
            good_chars.push(bad_char);
            good_chars.push('\n');
            good_chars.push('a');
            let x = super::strip_comments(&good_chars);
            assert!(x
                .chars()
                .all(|c| c == ' ' || c == '\t' || c == '\n' || c == 'a'));
            good_chars.pop();
            good_chars.pop();
            good_chars.pop();
        }
    }
}