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.

Sign up for GitHub

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

Jump to bottom

False positive with temporary_cstring_as_ptr #5838

Closed
kangalio opened this issue Jul 24, 2020 · 2 comments
Closed

False positive with temporary_cstring_as_ptr #5838

kangalio opened this issue Jul 24, 2020 · 2 comments
Labels
C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@kangalio
Copy link

kangalio commented Jul 24, 2020 •
edited
Loading

I tried this code:

fn some_function(data: *const u8) {}

fn main() {
    let string = "hello";
    some_function(CString::new(string).unwrap().as_ptr());
}

I expected no clippy complains about this one.

Instead, clippy gave "that pointer will be invalid outside this expression". Yes, "that pointer will be invalid outside this expression", but I only need it valid inside this expression, so this is a false positive.

Meta

Backtrace 

error: you are getting the inner pointer of a temporary `CString`
--> src/main.rs:7:19
|
7 |     some_function(CString::new(string).unwrap().as_ptr());
|                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[deny(clippy::temporary_cstring_as_ptr)]` on by default
= note: that pointer will be invalid outside this expression
help: assign the `CString` to a variable to extend its lifetime
--> src/main.rs:7:19
|
7 |     some_function(CString::new(string).unwrap().as_ptr());
|                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr
@kangalio kangalio added the C-bug Category: Clippy is not doing the correct thing label Jul 24, 2020
@flip1995
Copy link
Member

See also this comment: #4375 (comment)

The CString is dropped after the function call. This can be seen, when looking at the MIR of the above example (bb8+bb9, _8 is the CString)

MIR output 
// WARNING: This output format is intended for human consumers only
// and is subject to change without notice. Knock yourself out.
fn uaf(_1: &str) -> () {
    debug server_name => _1;             // in scope 0 at src/lib.rs:5:12: 5:23
    let mut _0: ();                      // return place in scope 0 at src/lib.rs:5:31: 5:31
    let _3: ();                          // in scope 0 at src/lib.rs:10:9: 10:60
    let mut _4: *const i8;               // in scope 0 at src/lib.rs:10:16: 10:59
    let mut _5: &std::ffi::CStr;         // in scope 0 at src/lib.rs:10:16: 10:50
    let _6: &std::ffi::CStr;             // in scope 0 at src/lib.rs:10:16: 10:50
    let mut _7: &std::ffi::CString;      // in scope 0 at src/lib.rs:10:16: 10:50
    let _8: std::ffi::CString;           // in scope 0 at src/lib.rs:10:16: 10:50
    let mut _9: std::result::Result<std::ffi::CString, std::ffi::NulError>; // in scope 0 at src/lib.rs:10:16: 10:41
    let mut _10: &str;                   // in scope 0 at src/lib.rs:10:29: 10:40
    scope 1 {
        let _2: std::string::String;     // in scope 1 at src/lib.rs:9:13: 9:14
        scope 2 {
            debug x => _2;               // in scope 2 at src/lib.rs:9:13: 9:14
            let _11: std::string::String; // in scope 2 at src/lib.rs:12:13: 12:14
            scope 3 {
                debug y => _11;          // in scope 3 at src/lib.rs:12:13: 12:14
            }
        }
    }

    bb0: {
        StorageLive(_2);                 // scope 1 at src/lib.rs:9:13: 9:14
        _2 = const <std::string::String as std::convert::From<&str>>::from(const "abc") -> bb2; // scope 1 at src/lib.rs:9:17: 9:36
                                         // ty::Const
                                         // + ty: fn(&str) -> std::string::String {<std::string::String as std::convert::From<&str>>::from}
                                         // + val: Value(Scalar(<ZST>))
                                         // mir::Constant
                                         // + span: src/lib.rs:9:17: 9:29
                                         // + user_ty: UserType(0)
                                         // + literal: Const { ty: fn(&str) -> std::string::String {<std::string::String as std::convert::From<&str>>::from}, val: Value(Scalar(<ZST>)) }
                                         // ty::Const
                                         // + ty: &str
                                         // + val: Value(Slice { data: Allocation { bytes: [97, 98, 99], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [7], len: Size { raw: 3 } }, size: Size { raw: 3 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 3 })
                                         // mir::Constant
                                         // + span: src/lib.rs:9:30: 9:35
                                         // + literal: Const { ty: &str, val: Value(Slice { data: Allocation { bytes: [97, 98, 99], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [7], len: Size { raw: 3 } }, size: Size { raw: 3 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 3 }) }
    }

    bb1 (cleanup): {
        resume;                          // scope 0 at src/lib.rs:5:1: 15:2
    }

    bb2: {
        StorageLive(_3);                 // scope 2 at src/lib.rs:10:9: 10:60
        StorageLive(_4);                 // scope 2 at src/lib.rs:10:16: 10:59
        StorageLive(_5);                 // scope 2 at src/lib.rs:10:16: 10:50
        StorageLive(_6);                 // scope 2 at src/lib.rs:10:16: 10:50
        StorageLive(_7);                 // scope 2 at src/lib.rs:10:16: 10:50
        StorageLive(_8);                 // scope 2 at src/lib.rs:10:16: 10:50
        StorageLive(_9);                 // scope 2 at src/lib.rs:10:16: 10:41
        StorageLive(_10);                // scope 2 at src/lib.rs:10:29: 10:40
        _10 = _1;                        // scope 2 at src/lib.rs:10:29: 10:40
        _9 = const std::ffi::CString::new::<&str>(move _10) -> [return: bb3, unwind: bb4]; // scope 2 at src/lib.rs:10:16: 10:41
                                         // ty::Const
                                         // + ty: fn(&str) -> std::result::Result<std::ffi::CString, std::ffi::NulError> {std::ffi::CString::new::<&str>}
                                         // + val: Value(Scalar(<ZST>))
                                         // mir::Constant
                                         // + span: src/lib.rs:10:16: 10:28
                                         // + user_ty: UserType(1)
                                         // + literal: Const { ty: fn(&str) -> std::result::Result<std::ffi::CString, std::ffi::NulError> {std::ffi::CString::new::<&str>}, val: Value(Scalar(<ZST>)) }
    }

    bb3: {
        StorageDead(_10);                // scope 2 at src/lib.rs:10:40: 10:41
        _8 = const std::result::Result::<std::ffi::CString, std::ffi::NulError>::unwrap(move _9) -> [return: bb5, unwind: bb4]; // scope 2 at src/lib.rs:10:16: 10:50
                                         // ty::Const
                                         // + ty: fn(std::result::Result<std::ffi::CString, std::ffi::NulError>) -> std::ffi::CString {std::result::Result::<std::ffi::CString, std::ffi::NulError>::unwrap}
                                         // + val: Value(Scalar(<ZST>))
                                         // mir::Constant
                                         // + span: src/lib.rs:10:42: 10:48
                                         // + literal: Const { ty: fn(std::result::Result<std::ffi::CString, std::ffi::NulError>) -> std::ffi::CString {std::result::Result::<std::ffi::CString, std::ffi::NulError>::unwrap}, val: Value(Scalar(<ZST>)) }
    }

    bb4 (cleanup): {
        drop(_2) -> bb1;                 // scope 1 at src/lib.rs:14:5: 14:6
    }

    bb5: {
        _7 = &_8;                        // scope 2 at src/lib.rs:10:16: 10:50
        _6 = const <std::ffi::CString as std::ops::Deref>::deref(move _7) -> [return: bb6, unwind: bb7]; // scope 2 at src/lib.rs:10:16: 10:50
                                         // ty::Const
                                         // + ty: for<'r> fn(&'r std::ffi::CString) -> &'r <std::ffi::CString as std::ops::Deref>::Target {<std::ffi::CString as std::ops::Deref>::deref}
                                         // + val: Value(Scalar(<ZST>))
                                         // mir::Constant
                                         // + span: src/lib.rs:10:16: 10:50
                                         // + literal: Const { ty: for<'r> fn(&'r std::ffi::CString) -> &'r <std::ffi::CString as std::ops::Deref>::Target {<std::ffi::CString as std::ops::Deref>::deref}, val: Value(Scalar(<ZST>)) }
    }

    bb6: {
        _5 = _6;                         // scope 2 at src/lib.rs:10:16: 10:50
        StorageDead(_9);                 // scope 2 at src/lib.rs:10:49: 10:50
        StorageDead(_7);                 // scope 2 at src/lib.rs:10:49: 10:50
        _4 = const std::ffi::CStr::as_ptr(move _5) -> [return: bb8, unwind: bb7]; // scope 2 at src/lib.rs:10:16: 10:59
                                         // ty::Const
                                         // + ty: for<'r> fn(&'r std::ffi::CStr) -> *const i8 {std::ffi::CStr::as_ptr}
                                         // + val: Value(Scalar(<ZST>))
                                         // mir::Constant
                                         // + span: src/lib.rs:10:51: 10:57
                                         // + literal: Const { ty: for<'r> fn(&'r std::ffi::CStr) -> *const i8 {std::ffi::CStr::as_ptr}, val: Value(Scalar(<ZST>)) }
    }

    bb7 (cleanup): {
        drop(_8) -> bb4;                 // scope 2 at src/lib.rs:10:60: 10:61
    }

    bb8: {
        StorageDead(_5);                 // scope 2 at src/lib.rs:10:58: 10:59
        _3 = const strcpy(move _4) -> [return: bb9, unwind: bb7]; // scope 2 at src/lib.rs:10:9: 10:60
                                         // ty::Const
                                         // + ty: unsafe extern "C" fn(*const i8) {strcpy}
                                         // + val: Value(Scalar(<ZST>))
                                         // mir::Constant
                                         // + span: src/lib.rs:10:9: 10:15
                                         // + literal: Const { ty: unsafe extern "C" fn(*const i8) {strcpy}, val: Value(Scalar(<ZST>)) }
    }

    bb9: {
        StorageDead(_4);                 // scope 2 at src/lib.rs:10:59: 10:60
        drop(_8) -> [return: bb10, unwind: bb4]; // scope 2 at src/lib.rs:10:60: 10:61
    }

    bb10: {
        StorageDead(_8);                 // scope 2 at src/lib.rs:10:60: 10:61
        StorageDead(_6);                 // scope 2 at src/lib.rs:10:60: 10:61
        StorageDead(_3);                 // scope 2 at src/lib.rs:10:60: 10:61
        StorageLive(_11);                // scope 2 at src/lib.rs:12:13: 12:14
        _11 = const <std::string::String as std::convert::From<&str>>::from(const "def") -> [return: bb11, unwind: bb4]; // scope 2 at src/lib.rs:12:17: 12:36
                                         // ty::Const
                                         // + ty: fn(&str) -> std::string::String {<std::string::String as std::convert::From<&str>>::from}
                                         // + val: Value(Scalar(<ZST>))
                                         // mir::Constant
                                         // + span: src/lib.rs:12:17: 12:29
                                         // + user_ty: UserType(2)
                                         // + literal: Const { ty: fn(&str) -> std::string::String {<std::string::String as std::convert::From<&str>>::from}, val: Value(Scalar(<ZST>)) }
                                         // ty::Const
                                         // + ty: &str
                                         // + val: Value(Slice { data: Allocation { bytes: [100, 101, 102], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [7], len: Size { raw: 3 } }, size: Size { raw: 3 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 3 })
                                         // mir::Constant
                                         // + span: src/lib.rs:12:30: 12:35
                                         // + literal: Const { ty: &str, val: Value(Slice { data: Allocation { bytes: [100, 101, 102], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [7], len: Size { raw: 3 } }, size: Size { raw: 3 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 3 }) }
    }

    bb11: {
        _0 = const ();                   // scope 1 at src/lib.rs:8:5: 14:6
                                         // ty::Const
                                         // + ty: ()
                                         // + val: Value(Scalar(<ZST>))
                                         // mir::Constant
                                         // + span: src/lib.rs:8:5: 14:6
                                         // + literal: Const { ty: (), val: Value(Scalar(<ZST>)) }
        drop(_11) -> [return: bb12, unwind: bb4]; // scope 2 at src/lib.rs:14:5: 14:6
    }

    bb12: {
        StorageDead(_11);                // scope 2 at src/lib.rs:14:5: 14:6
        drop(_2) -> bb13;                // scope 1 at src/lib.rs:14:5: 14:6
    }

    bb13: {
        StorageDead(_2);                 // scope 1 at src/lib.rs:14:5: 14:6
        return;                          // scope 0 at src/lib.rs:15:2: 15:2
    }
}

@flip1995 flip1995 added the E-medium Call for participation: Medium difficulty level problem and requires some initial experience. label Jul 29, 2020
wiomoc added a commit to wiomoc/rust-clippy that referenced this issue Aug 16, 2020
@wiomoc
ignore usage in same expr temporary_cstring_as_ptr
dc74fd0 
Fixes: rust-lang#5838
@jyn514 jyn514 mentioned this issue Sep 1, 2020
Merged
@phansch phansch added the I-false-positive Issue: The lint was triggered on code it shouldn't have label Dec 18, 2020
@Jarcho
Copy link
Contributor

Jarcho commented Jan 4, 2022

This should be closed as it's not in clippy anymore.

@llogiq llogiq closed this as completed Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@phansch @llogiq @Jarcho @flip1995 @kangalio