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

pinger argument unparsing unsoundness #238

Open
ijackson opened this issue Dec 15, 2022 · 1 comment
Open

pinger argument unparsing unsoundness #238

ijackson opened this issue Dec 15, 2022 · 1 comment

Comments

@ijackson
Copy link

For Reasons I was looking at the source code for this crate, and I found code like this:

    fn ping_args(&self, target: String) -> Vec<String> {
...
        if let Some(interface) = &self.interface {
            args.push("-I".into());
            args.push(interface.clone());
        }
        args.push(target);

This is unsound, because if target starts with a - it will be interpreted by ping as an option. (By unsound I mean this: if this API is passed untrusted input, undesirable malfunctions, which might be security relevant, can occur.)

I don't think this is very exploitable, on Linux at least, because almost all of the options also require a target, which when this malfunction occurs won't be supplied. The worst that can be done seems to be to pass -V or -h and cause the rest of the pinger library to try to parse the help or version output, which will be a malfunction, but fairly harmless.

I suggest that the ping_args function ought to take something like target: SyntaxCheckedTargetHost (with struct SyntaxCheckedTargetHost(String) and then in the common code you would check that the thing starts with an alphanumeric or [ or : when constructing the SyntaxCheckedTargetHost.

Thanks,.

@TieWay59
Copy link

You're right, the whole process of ping_with_interval doesn't guard the target input.

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