Lint str-ptr-in-c-abi discourage str pointers in C ABI fns

This discourages using a pointer to a `str` instead of a CString for
`extern "C"` functions.

Author: fpdotmonkey

CHANGELOG.md

@@ -7456,6 +7456,7 @@ Released 2018-09-13
 [`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
 [`std_instead_of_alloc`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_alloc
 [`std_instead_of_core`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_core
+[`str_ptr_in_c_abi`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_ptr_in_c_abi
 [`str_split_at_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_split_at_newline
 [`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string
 [`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add

clippy_lints/src/declared_lints.rs

@@ -715,6 +715,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::std_instead_of_core::ALLOC_INSTEAD_OF_CORE_INFO,
     crate::std_instead_of_core::STD_INSTEAD_OF_ALLOC_INFO,
     crate::std_instead_of_core::STD_INSTEAD_OF_CORE_INFO,
+    crate::str_ptr_in_c_abi::STR_PTR_IN_C_ABI_INFO,
     crate::string_patterns::MANUAL_PATTERN_CHAR_COMPARISON_INFO,
     crate::string_patterns::SINGLE_CHAR_PATTERN_INFO,
     crate::strings::STR_TO_STRING_INFO,

clippy_lints/src/lib.rs

@@ -349,6 +349,7 @@ mod size_of_in_element_count;
 mod size_of_ref;
 mod slow_vector_initialization;
 mod std_instead_of_core;
+mod str_ptr_in_c_abi;
 mod string_patterns;
 mod strings;
 mod strlen_on_c_strings;
@@ -868,6 +869,7 @@ rustc_lint::late_lint_methods!(
         RedundantElse: redundant_else::RedundantElse = redundant_else::RedundantElse,
         RestWhenDestructuringStruct: rest_when_destructuring_struct::RestWhenDestructuringStruct = rest_when_destructuring_struct::RestWhenDestructuringStruct,
         BlockScrutinee: block_scrutinee::BlockScrutinee = block_scrutinee::BlockScrutinee,
+        StrPtrInCAbi: str_ptr_in_c_abi::StrPtrInCAbi = str_ptr_in_c_abi::StrPtrInCAbi,
         // add late passes here, used by `cargo dev new_lint`
     ]]
 );

clippy_lints/src/str_ptr_in_c_abi.rs

@@ -0,0 +1,87 @@
+use rustc_hir::{Expr, ExprKind, TyKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// This lint triggers if a pointer to a Rust `str` is passed into an `extern "C"` interface
+    /// where you should instead be providing a pointer to a `CString`.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Foreign functions under the C ABI expect that a string ends with a null byte (`'\0'`).
+    /// Rust's `str` doesn't provide a null byte.  Instead it contains a length for the string.
+    /// Without a null byte, foreign functions can read beyond the memory allocated to the string searching for the null terminator, causing undefined behavior (UB).
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # unsafe extern "C" fn strlen(s: *const i8) -> usize { unimplemented!() }
+    /// unsafe { strlen("Hello".as_ptr() as *const _) };
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # unsafe extern "C" fn strlen(s: *const i8) -> usize { unimplemented!() }
+    /// let cstring = std::ffi::CString::new("Hello".as_bytes()).unwrap();
+    /// unsafe { strlen(cstring.as_ptr()) };
+    /// ```
+    #[clippy::version = "1.99.0"]
+    pub STR_PTR_IN_C_ABI,
+    suspicious,
+    "discourage str pointers in C ABI fns"
+}
+
+declare_lint_pass!(StrPtrInCAbi => [STR_PTR_IN_C_ABI]);
+
+impl<'tcx> LateLintPass<'tcx> for StrPtrInCAbi {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        // find call expressions
+        if let ExprKind::Call(callee, args) = expr.kind
+        // where the signature of the callee shows it is an `extern "C" fn`
+            && is_extern_c_fn(cx, callee)
+        // and the fn is given a str pointer as argument
+            && let spans = args
+                .iter()
+                .filter(|arg| is_str_ptr(cx, arg))
+                .map(|arg| arg.span)
+                .collect::<Vec<_>>()
+            && !spans.is_empty()
+        {
+            span_lint_and_help(
+                cx,
+                STR_PTR_IN_C_ABI,
+                spans,
+                "giving a pointer to a Rust `str` to an `extern \"C\" fn` can cause undefined behavior",
+                /* help_span */ None,
+                "if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that",
+            );
+        }
+    }
+}
+
+/// Does the expression represent an `extern "C" fn` of some type?
+fn is_extern_c_fn<'tcx>(cx: &LateContext<'tcx>, callee: &'tcx Expr<'tcx>) -> bool {
+    let callee_ty = cx.typeck_results().expr_ty_adjusted(callee);
+    if !(callee_ty.is_fn() || callee_ty.is_fn_ptr()) {
+        return false;
+    }
+    // NOTE: fn_sig panics if callee_ty isn't a function, but the above return ensures that it is
+    matches!(callee_ty.fn_sig(cx.tcx).abi(), rustc_abi::ExternAbi::C { .. })
+}
+
+/// Is `arg` shaped like `derefs_to_str.as_ptr() as *const _`, maybe without the cast?
+fn is_str_ptr<'tcx>(cx: &LateContext<'tcx>, arg: &Expr<'tcx>) -> bool {
+    let typeck = cx.typeck_results();
+    match arg.kind {
+        // NOTE: recursion is bounded by how many nested casts to ptr the user does
+        ExprKind::Cast(expr, ty) if matches!(ty.kind, TyKind::Ptr(_)) => is_str_ptr(cx, expr),
+        ExprKind::MethodCall(method, this, _args, _span) => {
+            matches!(method.ident.name, sym::as_ptr | sym::as_mut_ptr)
+                && typeck.expr_ty_adjusted(this).peel_refs().is_str()
+        },
+        _ => false,
+    }
+}

clippy_utils/src/sym.rs

@@ -138,6 +138,7 @@ generate! {
     as_deref,
     as_deref_mut,
     as_mut,
+    as_mut_ptr,
     as_path,
     as_ptr,
     as_str,

tests/ui/str_ptr_in_c_abi.rs

@@ -0,0 +1,70 @@
+#![warn(clippy::str_ptr_in_c_abi)]
+
+use std::ffi::CString;
+
+fn main() {
+    // This should use a pointer to a CString
+    unsafe { printf("Hello".as_ptr() as *const _) };
+    //~^ str_ptr_in_c_abi
+
+    // Like this
+    let cstring = CString::new("Hello".as_bytes()).unwrap();
+    unsafe { printf(cstring.as_ptr()) };
+
+    // One should also use mut pointers to CStrings
+    let mut buffer = String::new();
+    let mut buffer = buffer.as_mut_str(); // this lint can only detect `str`s for now
+    unsafe { strcpy(buffer.as_mut_ptr() as *mut _, cstring.as_ptr()) };
+    //~^ str_ptr_in_c_abi
+
+    let mut cstring_mut = CString::new([]).unwrap();
+    unsafe { strcpy(cstring_mut.into_raw(), cstring.as_ptr()) };
+
+    // Two rust strings at once!
+    unsafe { strcpy(buffer.as_mut_ptr() as *mut _, "Hello".as_ptr() as *const _) };
+    //~^ str_ptr_in_c_abi
+
+    // It can detect smart pointers to str
+    let hello_string: String = "Hello".into();
+    let hello_box: Box<str> = "Hello".into();
+    let hello_rc: std::rc::Rc<str> = "Hello".into();
+    unsafe { printf(hello_string.as_ptr() as *const _) };
+    //~^ str_ptr_in_c_abi
+    unsafe { printf(hello_box.as_ptr() as *const _) };
+    //~^ str_ptr_in_c_abi
+    unsafe { printf(hello_rc.as_ptr() as *const _) };
+    //~^ str_ptr_in_c_abi
+
+    // It detects str to ptr casts in variadics
+    let fmt = CString::new("%s\n".as_bytes()).unwrap();
+    unsafe { printf(fmt.as_ptr(), "I'm (incorrectly) printf-ing a str!".as_ptr() as *const _) };
+    //~^ str_ptr_in_c_abi
+
+    // The cast isn't necessary to trigger the lint, which is relevant for not type-checked variadics
+    unsafe { printf(fmt.as_ptr(), "hello".as_ptr()) };
+    //~^ str_ptr_in_c_abi
+
+    // It detects the str ptr through multiple casts
+    unsafe { printf("hello".as_ptr() as *const _ as *const _) };
+    //~^ str_ptr_in_c_abi
+    unsafe { printf("hello".as_ptr() as *const _ as *const _ as *const _) };
+    //~^ str_ptr_in_c_abi
+
+    // it can see inside of smart pointers to functions
+    let printf_box: Box<unsafe extern "C" fn(*const i8, ...) -> i32> = Box::new(printf);
+    let printf_rc: std::rc::Rc<unsafe extern "C" fn(*const i8, ...) -> i32> = std::rc::Rc::new(printf);
+    unsafe { printf_box("hello".as_ptr() as *const _) };
+    //~^ str_ptr_in_c_abi
+    unsafe { printf_rc("hello".as_ptr() as *const _) };
+    //~^ str_ptr_in_c_abi
+    #[allow(clippy::needless_borrow)]
+    unsafe {
+        (&printf)("hello".as_ptr() as *const _)
+        //~^ str_ptr_in_c_abi
+    };
+}
+
+unsafe extern "C" {
+    fn strcpy(dst: *mut i8, src: *const i8) -> *mut i8;
+    fn printf(format: *const i8, ...) -> i32;
+}

tests/ui/str_ptr_in_c_abi.stderr

@@ -0,0 +1,108 @@
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:7:21
+   |
+LL |     unsafe { printf("Hello".as_ptr() as *const _) };
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+   = note: `-D clippy::str-ptr-in-c-abi` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::str_ptr_in_c_abi)]`
+
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:17:21
+   |
+LL |     unsafe { strcpy(buffer.as_mut_ptr() as *mut _, cstring.as_ptr()) };
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:24:21
+   |
+LL |     unsafe { strcpy(buffer.as_mut_ptr() as *mut _, "Hello".as_ptr() as *const _) };
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:31:21
+   |
+LL |     unsafe { printf(hello_string.as_ptr() as *const _) };
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:33:21
+   |
+LL |     unsafe { printf(hello_box.as_ptr() as *const _) };
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:35:21
+   |
+LL |     unsafe { printf(hello_rc.as_ptr() as *const _) };
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:40:35
+   |
+LL |     unsafe { printf(fmt.as_ptr(), "I'm (incorrectly) printf-ing a str!".as_ptr() as *const _) };
+   |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:44:35
+   |
+LL |     unsafe { printf(fmt.as_ptr(), "hello".as_ptr()) };
+   |                                   ^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:48:21
+   |
+LL |     unsafe { printf("hello".as_ptr() as *const _ as *const _) };
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:50:21
+   |
+LL |     unsafe { printf("hello".as_ptr() as *const _ as *const _ as *const _) };
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:56:25
+   |
+LL |     unsafe { printf_box("hello".as_ptr() as *const _) };
+   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:58:24
+   |
+LL |     unsafe { printf_rc("hello".as_ptr() as *const _) };
+   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+
+error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
+  --> tests/ui/str_ptr_in_c_abi.rs:62:19
+   |
+LL |         (&printf)("hello".as_ptr() as *const _)
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if the foreign function calls for a null-terminated string in this position, first convert this value to a `std::ffi::CString` and take the pointer from that
+
+error: aborting due to 13 previous errors
+