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

CHANGELOG.md

@@ -7455,6 +7455,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

@@ -714,6 +714,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

@@ -348,6 +348,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;
@@ -866,6 +867,7 @@ rustc_lint::late_lint_methods!(
         RefPatterns: ref_patterns::RefPatterns = ref_patterns::RefPatterns,
         RedundantElse: redundant_else::RedundantElse = redundant_else::RedundantElse,
         RestWhenDestructuringStruct: rest_when_destructuring_struct::RestWhenDestructuringStruct = rest_when_destructuring_struct::RestWhenDestructuringStruct,
+        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,91 @@
+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.
+    /// This leads to two problems.
+    ///
+    /// 1. The length parameter of the Rust string will be misinterpreted as a character, which is logically invalid.
+    /// 2. Without a null byte, foreign functions will 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,
+    nursery,
+    "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
+        // extern "C" fn
+            && is_extern_c_fn(cx, callee)
+        // and takes a raw pointer as an argument
+        // and to that argument gives a pointer to a Rust `str`
+            && let span = args
+                .iter()
+                .filter(|arg| is_cast_str_ptr_to_raw_ptr(cx, arg))
+                .map(|arg| arg.span)
+                .collect::<Vec<_>>()
+            && !span.is_empty()
+        {
+            span_lint_and_help(
+                cx,
+                STR_PTR_IN_C_ABI,
+                span,
+                "giving a pointer to a Rust `str` to an `extern \"C\" fn` can cause undefined behavior",
+                /* help_span */ None,
+                "first convert the `str` to a `std::ffi::CString` and then get a pointer from there",
+            );
+        }
+    }
+}
+
+/// 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(callee);
+    if !(callee_ty.is_fn() || callee_ty.is_fn_ptr()) {
+        return false;
+    }
+    matches!(callee_ty.fn_sig(cx.tcx).abi(), rustc_abi::ExternAbi::C { .. })
+}
+
+/// If `arg` is `derefs_to_str.as(_mut)_ptr() as *const(mut) _`, then true.
+fn is_cast_str_ptr_to_raw_ptr<'tcx>(cx: &LateContext<'tcx>, arg: &Expr<'tcx>) -> bool {
+    let typeck = cx.typeck_results();
+    if let ExprKind::Cast(expr, ty) = arg.kind
+        && matches!(ty.kind, TyKind::Ptr(_))
+        && let ExprKind::MethodCall(method, this, _args, _span) = expr.kind
+        && matches!(method.ident.name, sym::as_ptr | sym::as_mut_ptr)
+        && typeck.expr_ty_adjusted(this).peel_refs().is_str()
+    {
+        return true;
+    }
+    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,47 @@
+#![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
+}
+
+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,60 @@
+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: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
+   = 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: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
+
+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: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
+
+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: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
+
+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: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
+
+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: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
+
+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: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
+
+error: aborting due to 7 previous errors
+