Rollup merge of rust-lang#33554 - sfackler:no-current-exe, r=alexcric…

…hton

Don't use env::current_exe with libbacktrace

If the path we give to libbacktrace doesn't actually correspond to the
current process, libbacktrace will segfault *at best*.

cc rust-lang#21889

r? @alexcrichton
cc @semarie

src/libstd/sys/common/gnu/libbacktrace.rs

@@ -15,7 +15,6 @@ use sys_common::backtrace::{output, output_fileline};
 
 pub fn print(w: &mut Write, idx: isize, addr: *mut libc::c_void,
              symaddr: *mut libc::c_void) -> io::Result<()> {
-    use env;
     use ffi::CStr;
     use ptr;
 
@@ -110,46 +109,22 @@ pub fn print(w: &mut Write, idx: isize, addr: *mut libc::c_void,
     // that is calculated the first time this is requested. Remember that
     // backtracing all happens serially (one global lock).
     //
-    // An additionally oddity in this function is that we initialize the
-    // filename via self_exe_name() to pass to libbacktrace. It turns out
-    // that on Linux libbacktrace seamlessly gets the filename of the
-    // current executable, but this fails on freebsd. by always providing
-    // it, we make sure that libbacktrace never has a reason to not look up
-    // the symbols. The libbacktrace API also states that the filename must
-    // be in "permanent memory", so we copy it to a static and then use the
-    // static as the pointer.
+    // Things don't work so well on not-Linux since libbacktrace can't track
+    // down that executable this is. We at one point used env::current_exe but
+    // it turns out that there are some serious security issues with that
+    // approach.
     //
-    // FIXME: We also call self_exe_name() on DragonFly BSD. I haven't
-    //        tested if this is required or not.
+    // Specifically, on certain platforms like BSDs, a malicious actor can cause
+    // an arbitrary file to be placed at the path returned by current_exe.
+    // libbacktrace does not behave defensively in the presence of ill-formed
+    // DWARF information, and has been demonstrated to segfault in at least one
+    // case. There is no evidence at the moment to suggest that a more carefully
+    // constructed file can't cause arbitrary code execution. As a result of all
+    // of this, we don't hint libbacktrace with the path to the current process.
     unsafe fn init_state() -> *mut backtrace_state {
         static mut STATE: *mut backtrace_state = ptr::null_mut();
-        static mut LAST_FILENAME: [libc::c_char; 256] = [0; 256];
         if !STATE.is_null() { return STATE }
-        let selfname = if cfg!(target_os = "freebsd") ||
-            cfg!(target_os = "dragonfly") ||
-            cfg!(target_os = "bitrig") ||
-            cfg!(target_os = "openbsd") ||
-            cfg!(target_os = "windows") {
-                env::current_exe().ok()
-            } else {
-                None
-            };
-        let filename = match selfname.as_ref().and_then(|s| s.to_str()) {
-            Some(path) => {
-                let bytes = path.as_bytes();
-                if bytes.len() < LAST_FILENAME.len() {
-                    let i = bytes.iter();
-                    for (slot, val) in LAST_FILENAME.iter_mut().zip(i) {
-                        *slot = *val as libc::c_char;
-                    }
-                    LAST_FILENAME.as_ptr()
-                } else {
-                    ptr::null()
-                }
-            }
-            None => ptr::null(),
-        };
-        STATE = backtrace_create_state(filename, 0, error_cb,
+        STATE = backtrace_create_state(ptr::null(), 0, error_cb,
                                        ptr::null_mut());
         STATE
     }

src/test/run-pass/backtrace-debuginfo.rs

@@ -32,11 +32,15 @@ macro_rules! dump_and_die {
     ($($pos:expr),*) => ({
         // FIXME(#18285): we cannot include the current position because
         // the macro span takes over the last frame's file/line.
-        if cfg!(target_os = "macos") ||
-           cfg!(target_os = "ios") ||
-           cfg!(target_os = "android") ||
-           cfg!(all(target_os = "linux", target_arch = "arm")) ||
-           cfg!(all(windows, target_env = "gnu")) {
+        if cfg!(any(target_os = "macos",
+                    target_os = "ios",
+                    target_os = "android",
+                    all(target_os = "linux", target_arch = "arm"),
+                    target_os = "windows",
+                    target_os = "freebsd",
+                    target_os = "dragonfly",
+                    target_os = "bitrig",
+                    target_os = "openbsd")) {
             // skip these platforms as this support isn't implemented yet.
         } else {
             dump_filelines(&[$($pos),*]);