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

The number of feature args could be incorrect #485

Open
waltdisgrace opened this issue Jul 16, 2019 · 2 comments
Open

The number of feature args could be incorrect #485

waltdisgrace opened this issue Jul 16, 2019 · 2 comments

Comments

@waltdisgrace
Copy link
Contributor

waltdisgrace commented Jul 16, 2019

If the user-provided number of feature args is incorrect, then the call to the parse_feature_args function, which utilizes this number, could lead to an out-of-bounds error that fails the program at runtime.

@mulkieran
Copy link
Member

@waltdisgrace Please generalize this issue so it covers the general case where the value for the number of feature args can be incorrect and lead to an out-of-bounds error. It describes the code in most, or possibly all, of the targets implemented.

@waltdisgrace waltdisgrace changed the title Refactor parse_feature_args function call The number of feature args could be incorrect Jul 26, 2019
@mulkieran
Copy link
Member

Some notes:

What you really want to look for are parse::<Type> lines, as with: https://github.com/stratis-storage/devicemapper-rs/blob/master/src/lineardev.rs#L438. Note that this in the method from_raw_table. This is where the entire table is parsed from the test result returned by the ioctl. The method parse, exists because LinearDevTargetParams implements FromStr: https://github.com/stratis-storage/devicemapper-rs/blob/master/src/lineardev.rs#L362. This method invokes parse twice, one for FlakeyTargetParams and one for LinearTargetParams. LinearTargetParams implements FromStr but is too simple to have features. FlakeyTargetParams implements FromStr, and does have features. At https://github.com/stratis-storage/devicemapper-rs/blob/master/src/lineardev.rs#L302 the first argument parsed is the number of features args. The problems is that then we trust that number unhesitatingly. In particular, we use that number to select a slice from the remaining list of items. If that number is actually longer than the list of items, our code will crash. You can try that easily, with the Rust playground, using something like this:

fn main() {
    let v = vec![1, 2, 3, 4];
    let d = &v[2..5];
}

You will find that the same problem shows up in most but maybe not all of our implmentations of FromStr for other devices. For example:

https://github.com/stratis-storage/devicemapper-rs/blob/master/src/thinpooldev.rs#L92

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

No branches or pull requests

2 participants