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

UB in get_columns_and_bytes_before #45

Closed
0xd34d10cc opened this issue Jan 8, 2020 · 3 comments · Fixed by #50
Closed

UB in get_columns_and_bytes_before #45

0xd34d10cc opened this issue Jan 8, 2020 · 3 comments · Fixed by #50
Labels

Comments

@0xd34d10cc
Copy link

The following code:

use nom_locate::LocatedSpan;

fn main() {
    let fragment = [b'a', b'b', b'c', b'd'];
    let span = LocatedSpan {
        offset: 12345,
        line: 0,
        fragment: std::str::from_utf8(&fragment).unwrap(),
        extra: ()
    };

    println!("{}", span.get_utf8_column());
}

prints random garbage value.

The problem is here:

nom_locate/src/lib.rs

Lines 216 to 223 in 504e849

let before_self = unsafe {
assert!(
self.offset <= isize::max_value() as usize,
"offset is too big"
);
let orig_input_ptr = self_ptr.offset(-(self.offset as isize));
slice::from_raw_parts(orig_input_ptr, self.offset)
};

There is no guarantee that self.offset bytes before self.fragment.as_bytes().as_ptr() are actually readable.

@progval progval added the bug label Jan 8, 2020
@progval
Copy link
Collaborator

progval commented Jan 8, 2020

Can this happen without constructing a LocatedSpan manually? I don't see any reason to do that; we should probably add an empty private field so it doesn't happen.

@0xd34d10cc
Copy link
Author

0xd34d10cc commented Jan 8, 2020

Can this happen without constructing a LocatedSpan manually?

Well, offset field is public, nothing prevents users from changing its value after construction.

@progval
Copy link
Collaborator

progval commented Jan 8, 2020

indeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants