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

Rust stack backtrace filled with unknown on Windows 8.1 #33985

Closed
skiwi2 opened this issue May 31, 2016 · 30 comments
Closed

Rust stack backtrace filled with unknown on Windows 8.1 #33985

skiwi2 opened this issue May 31, 2016 · 30 comments
Labels
O-windows-gnu Toolchain: GNU, Operating system: Windows

Comments

@skiwi2
Copy link

skiwi2 commented May 31, 2016

When running a piece of code I get the following stack backtrace:

' panicked at 'index out of bounds', ../src/libcore\option.rs:700 stack backtrace: 0: 0x4707c3 - 1: 0x46fcb5 - 2: 0x433baf - 3: 0x43573b - 4: 0x462eab - 5: 0x484e05 - 6: 0x4a61c4 - 7: 0x41ffdb - 8: 0x41ff4c - 9: 0x402bf5 - 10: 0x46f2ed - 11: 0x46f11a - 12: 0x42ba1a - 13: 0x4013b4 - 14: 0x4014e7 - 15: 0x7ffa1a5613d1 - error: Process didn't exit successfully: `C:\Users\Frank\Dropbox\workspace\sieve_multithreaded\target\debug\sieve_multithreaded.exe` (exit code: 101)

I'm running cargo build with RUST_BACKTRACE=1 and have the following %PATH%:

C:\Program Files\Microsoft MPI\Bin;C:\Program Files\Haskell\bin;C:\Program Files\Haskell Platform\7.10.2-a\lib\extralibs\bin;C:\Program Files\Haskell Platform
7.10.2-a\bin;C:\ProgramData\Oracle\Java\javapath;C:\jet9.0-eval-amd64\bin;C:\Program Files\ImageMagick-6.8.7-Q16;C:\Windows\system32;C:\Windows;C:\Windows\Syste
m32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Program Files\Microsoft\Web Platform Installer;C:\Program Files (x86)\Microsoft ASP.NET\ASP.NET Web Pag
es\v1.0;C:\Program Files\Microsoft SQL Server\110\Tools\Binn;C:\Program Files (x86)\CMake 2.8\bin;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\W
INDOWS\System32\WindowsPowerShell\v1.0;C:\Program Files (x86)\Windows Live\Shared;C:\Program Files\Java\jdk1.7.0_25\bin;C:\Program Files (x86)\QuickTime\QTSys
tem;C:\Program Files (x86)\Tesseract-OCR;C:\Program Files\Calibre2;C:\Program Files (x86)\Windows Kits\8.1\Windows Performance Toolkit;C:\Program Files (x86)
\Microsoft SDKs\TypeScript\1.0;C:\Program Files (x86)\Subversion\bin;C:\texlive\2014\bin\win32;C:\Program Files\nodejs;C:\HashiCorp\Vagrant\bin;C:\Program Fil
es (x86)\Gource\cmd;C:\Program Files (x86)\NVIDIA Corporation\PhysX\Common;C:\Program Files\Haskell Platform\7.10.2-a\mingw\bin;C:\Leksah\bin;C:\Program Files
(x86)\Skype\Phone;C:\Program Files\Rust stable GNU 1.9\bin;C:\Users\Frank\AppData\Roaming\cabal\bin;C:\Program Files (x86)\NVIDIA Corporation\PhysX\Common;C:\P
rogramData\Oracle\Java\javapath;C:\jet9.0-eval-amd64\bin;C:\Program Files\ImageMagick-6.8.7-Q16;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windo
ws\System32\WindowsPowerShell\v1.0;C:\Program Files\Microsoft\Web Platform Installer;C:\Program Files (x86)\Microsoft ASP.NET\ASP.NET Web Pages\v1.0;C:\Progr
am Files\Microsoft SQL Server\110\Tools\Binn;C:\Program Files (x86)\CMake 2.8\bin;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32\W
indowsPowerShell\v1.0;C:\Program Files (x86)\Windows Live\Shared;C:\Program Files\Java\jdk1.7.0_25\bin;C:\Program Files (x86)\QuickTime\QTSystem;C:\Program F
iles (x86)\Tesseract-OCR;C:\Program Files\Calibre2;C:\Program Files (x86)\Windows Kits\8.1\Windows Performance Toolkit;C:\Program Files (x86)\Microsoft SDKs\T
ypeScript\1.0;C:\Program Files (x86)\Subversion\bin;C:\Program Files (x86)\Skype\Phone;C:\python27;C:\python27\Scripts;D:\texlive\2014\bin\win32;C:\Users\Fran
k\AppData\Roaming\npm;C:\Program Files\Oracle\VirtualBox;C:\grails-3.0.9\bin;C:\Program Files\mingw-w64\x86_64-4.9.0-posix-seh-rt_v3-rev2\mingw64\bin;C:\Program
Files (x86)\Git\bin;C:\cntk

I have no clue why it is not showing more information about the stack frames and haven't had any success in Googling it.

@petrochenkov
Copy link
Contributor

It's a very recent regression (couple of weeks maybe), I observe it too.

@alexcrichton
Copy link
Member

@skiwi2 do you have some sample code that could be used to reproduce this?

@skiwi2
Copy link
Author

skiwi2 commented May 31, 2016

I'm running the following code: https://gist.github.com/skiwi2/ce188c7d3b13fe3e3dbdb5c5f0752cd1 (Please don't mind the quality)

@alexcrichton
Copy link
Member

I was unfortunately unable to reproduce, @skiwi2 can you also gist the output of rustc -vV as well as cargo -V?

@alexcrichton
Copy link
Member

Ah and also, the panic message (e.g. the full output of one compile) would also be useful

@skiwi2
Copy link
Author

skiwi2 commented May 31, 2016

@alexcrichton What exactly do you mean by your last request? Do you want the program output (including logging statements), or some compilation output?

@skiwi2
Copy link
Author

skiwi2 commented May 31, 2016

The rustc and Cargo info files are up at https://gist.github.com/skiwi2/be1a8bf2171f3c1114a5fa65933e6732

@alexcrichton
Copy link
Member

@skiwi2 oh just along the lines of gisting the entire error (from the invocation of rustc/cargo all the way through to the next prompt). It looks like the snippet above may just be a subset? The panic message may be enough, but can't hurt to have more output!

@retep998
Copy link
Member

#33554 was merged on May 14, your rustc is from May 18, could that have caused the regression perhaps?

@alexcrichton
Copy link
Member

Oh! I apologize, I thought this was about an ICE not for a normal panic (which this appears to be about). In that case, yes, it's likely closely related to #33554, which was implemented for security purposes.

@retep998
Copy link
Member

In the meantime if you do want backtraces and don't mind installing VC++ you can always use the x86_64-pc-windows-msvc version of Rust.

@nagisa nagisa added the O-windows-gnu Toolchain: GNU, Operating system: Windows label Jun 1, 2016
@DoumanAsh
Copy link

Having the same issue on current stable gnu toolchain

@abonander
Copy link
Contributor

This is still occurring on recent nightlies: https://www.reddit.com/r/rust/comments/4zgzfk/is_there_an_unwrap_in_macro_form/d6vrvuw

@ruabmbua
Copy link
Contributor

ruabmbua commented Sep 13, 2016

I have the same problem too on newest gnu nightly and stable.
Could someone look into fixing that? It is quiet annoying, when I have to use gdb for my cargo test cases, to actually find the panicing code.

Edit:
msvc is not an option for me

@petrochenkov
Copy link
Contributor

This is a stable -> stable regression, by the way, if it helps with prioritization.

@Moraxes
Copy link

Moraxes commented Oct 19, 2016

This issue affects me as well. It makes debugging really hard. Luckily I can switch to msvc for now, but I have to abandon most of my tooling (e.g. gdb, which AFAIK doesn't support msvc-built binaries; I also often link to mingw-built libraries).

@petrochenkov
Copy link
Contributor

I've confirmed, #33554 is indeed the PR that killed backtraces (cc @sfackler). I'll try to investigate further.

@petrochenkov
Copy link
Contributor

petrochenkov commented Oct 20, 2016

@MoreAxes
This issue doesn't affect debugging with gdb, only backtraces produced by rustc.
So, you still can run the executable with gdb, break on rust_panic and obtain the backtrace with bt.

@petrochenkov
Copy link
Contributor

The simplest solution is to revert #33554, but use env::current_exe (aka GetModuleFileNameW) only on Windows.
Having backtraces seems more useful than guarding from unconfirmed vulnerability.

@petrochenkov
Copy link
Contributor

One of concerns from #21889 (in addition to malicious executable replacement) were race conditions in env::current_exe. GetModuleFileNameW seems to not be racy according to various people on the internet.

@sfackler
Copy link
Member

If the windows implementation doesn't have those race issues then it definitely seems reasonable to just turn this back on on Windows.

@retep998
Copy link
Member

retep998 commented Oct 21, 2016

@petrochenkov The point is not whether env::current_exe is safe (which it definitely is on Windows), but rather the fact that the file that env::current_exe points to may not be the original executable, but some other file (yes, this can happen on Windows). This other file can be carefully crafted to exploit certain bugs in libbacktrace potentially allowing an exploit. Granted, this exploit requires the attacker to have a rather extreme amount of power in the first place to swap files like that, but that's the reasoning used for that PR.

@arielb1
Copy link
Contributor

arielb1 commented Oct 21, 2016

This vulnerability is hard to reach, but I don't want to be introducing random vulnerabilities to all Rust programs.

@petrochenkov
Copy link
Contributor

petrochenkov commented Oct 21, 2016

You assume feeding an arbitrary ill-formed PE/COFF to libbacktrace is indeed a vulnerability, I haven't seen this confirmed so far. By default it sounds like "feeding an arbitrary text to rustc is vulnerability".
Anyway, I'll try to investigate why exactly libbacktrace needs these paths to produce backtraces and maybe find another solution before submitting a PR reverting #33554.

@petrochenkov
Copy link
Contributor

petrochenkov commented Oct 22, 2016

Update:

Prelude

DWARF debug info is used on Windows/GNU, as opposed to PDB debug info on Windows/MSVC.
WinAPI has tools for reading PDB, but can't read DWARF. libbacktrace can read DWARF, that's why libbacktrace is used on Windows/GNU but not on Windows/MSVC.
Debug info enriches backtraces with file names, line numbers, inlined functions etc in addition to the minimal backtrace - non-inlined function names and addresses, that can be obtained with WinAPI alone without using libbacktrace.

The problem

libbacktrace needs to the read the executable file to extract DWARF debug info from it.
If filename is not provided to libbacktrace in backtrace_create_state (this is what #33554 did), then libbacktrace will try to retrieve it itself using getexecname, /proc/self/exe or /proc/curproc/file, whatever works first. Neither works on Windows/GNU, therefore backtraces are broken.

In any case, the executable file is read and interpreted by libbacktrace whether we provide the filename to backtrace_create_state or not, unless some error happens. There's still a window between the file name is retrieved and the file is read, so the hypothetical vulnerability discussed above is inherent to libbacktrace, but the attack window is much smaller because libbactrace falls back to getexecname etc almost immediately before reading the file and don't cache the obtained filename for later use.

So, why don't we patch libbacktrace like this commit does and use GetModuleFileName (instead of /proc/self/exe etc) to get the fallback name on Windows? This fixes backtraces for sure.
The answer is that GetModuleFileName returns the original executable name with which it was launched. If executable is renamed and replaced with a malicious file, then GetModuleFileName will refer to the malicious file even if it's called after renaming.

Possible solutions

  • Try to use QueryFullProcessImageName instead of GetModuleFileName, it should support renaming (this is what Fix backtraces on Windows/GNU #37359 does).
  • Use minimal backtraces on Windows/GNU without additional goodies like line numbers, they doesn't require libbacktrace at all.
  • Use GetModuleFileName and enjoy the hypothetical vulnerability (i.e. revert Don't use env::current_exe with libbacktrace #33554 for Windows).
  • Rewrite libbacktrace in Rust and harden it against ill-formed malicious DWARVes.

@retep998
Copy link
Member

Use minimal backtraces on Windows/GNU without additional goodies like line numbers, they doen't require libbacktrace at all.

Note that the names of statically linked functions are also only specified in the debuginfo. Without debuginfo all you get is the names of functions that are exported from a DLL, and because Rust statically links everything by default, this means all Rust functions end up as unnamed, not even a mangled name.

@petrochenkov
Copy link
Contributor

petrochenkov commented Oct 25, 2016

Regarding @alexcrichton 's alternatives from #37359 (comment)

Figure out another method of giving libbacktrace dwarf data

Unless there's a way to somehow automatically read DWARF into a process' memory with OS' help, I don't see a way to avoid reading it from a file.

Figure out another method of backtraces on MinGW

This means either even more invasive changes in libbacktraces to make it secure enough (how to determine if it's secure enough?), or find a readily available more secure version of libbacktrace (any concrete variants? how to determine if the alternative is secure enough?), or rewrite libbacktrace by ourselves (how to determine if our rewrite is secure enough?). Why do we think libbacktrace is not secure enough? I haven't seen any proofs that unintentional code-execution is possible yet.
Meanwhile, a tier1 platform doesn't have backtraces for almost half a year.

segevfiner added a commit to segevfiner/rust that referenced this issue Jan 24, 2017
This is done by adding a function that can return a filename
to pass to backtrace_create_state. The filename is obtained in
a safe way by first getting the filename, locking the file so it can't
be moved, and then getting the filename again and making sure it's the same.

See: rust-lang#37359 (comment)
Issue: rust-lang#33985
bors added a commit that referenced this issue Jan 28, 2017
…trochenkov

Make backtraces work on Windows GNU targets again.

This is done by adding a function that can return a filename
to pass to backtrace_create_state. The filename is obtained in
a safe way by first getting the filename, locking the file so it can't
be moved, and then getting the filename again and making sure it's the same.

See: #37359 (comment)
Issue: #33985

Note though that this isn't that pretty...

I had to implement a `WideCharToMultiByte` wrapper function to convert to the ANSI code page. This will work better than only allowing ASCII provided that the ANSI code page is set to the user's local language, which is often the case.

Also, please make sure that I didn't break the Unix build.
@petrochenkov
Copy link
Contributor

Fixed by #39234

@dpelevin
Copy link

dpelevin commented Feb 2, 2017

@petrochenkov, with current implementation application crashes in the case of panic, if debug info is stripped out with (strip -s helloworld.exe) and RUST_BACKTRACE is set to 1.

@petrochenkov
Copy link
Contributor

@dpelevin
It's better to create a new issue for this problem, so it doesn't get lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows-gnu Toolchain: GNU, Operating system: Windows
Projects
None yet
Development

No branches or pull requests