Skip to content

/rust

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

Don't use env::current_exe with libbacktrace #33554

Merged
merged 1 commit into from May 14, 2016

Conversation

Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
beta-accepted
Projects
None yet
Milestone
No milestone
6 participants
@sfackler @semarie @alexcrichton @nagisa @bors @hoodie
@sfackler
Copy link
Member

sfackler commented May 11, 2016

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

cc #21889

r? @alexcrichton
cc @semarie

@rust-highfive rust-highfive assigned alexcrichton May 11, 2016

src/libstd/sys/common/gnu/libbacktrace.rs Outdated
// down that executable this is. We can't use env::current_exe because
// libbacktrace will break in unboundedly horrible ways if the file at that
// path doesn't actually correspond to this process at the time we look at
// it.

This comment has been minimized.

Sign in to view
Copy link
@alexcrichton

alexcrichton May 11, 2016

Member

Could you elaborate here and indicate that it's a security concern for not reading an arbitrary executable? Basically emphasize the facts of:

  • You can probably trick current_exe to point at any file, or rather that's the worst case.
  • There's been at least one bug reported in libbacktrace for corrupt dwarf info
  • We don't know of any other vulnerabilities, but at worse we assume that arbitrary code execution is possible with corrupt dwarf info to libbacktrace (until we find evidence to the contrary)

And thus we don't pass a pathname to libbacktrace
@semarie

This comment has been minimized.

Sign in to view
Copy link
Contributor

semarie commented May 11, 2016

I agree with the comment of @alexcrichton.

I expect also some tests should be marked ignored for some platforms (like run-pass/backtrace.rs for example).

@sfackler sfackler force-pushed the sfackler:no-current-exe branch from f07835c to 9c2e691 May 11, 2016

@sfackler

This comment has been minimized.

Sign in to view
Copy link
Member Author

sfackler commented May 11, 2016

Updated. I fixed run-pass/backtrace-debuginfo.rs - I guess we'll let the builders find out what other tests need to be fixed.
@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member

alexcrichton commented May 11, 2016

@bors: r+ 9c2e691

Manishearth added a commit to Manishearth/rust that referenced this pull request May 12, 2016

@Manishearth
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
5f90f49

@Manishearth Manishearth referenced this pull request May 12, 2016

Closed

Rollup of 11 pull requests #33582

bors added a commit that referenced this pull request May 12, 2016

@bors
Auto merge of #33582 - Manishearth:rollup, r=Manishearth 
Rollup of 11 pull requests

- Successful merges: #33450, #33531, #33532, #33541, #33544, #33552, #33554, #33555, #33560, #33563, #33566
- Failed merges:
1d50670

eddyb added a commit to eddyb/rust that referenced this pull request May 12, 2016

@eddyb
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
9d77943

@eddyb eddyb referenced this pull request May 12, 2016

Closed

Rollup of 15 pull requests #33589

bors added a commit that referenced this pull request May 12, 2016

@bors
Auto merge of #33589 - eddyb:rollup, r=eddyb 
Rollup of 15 pull requests

- Successful merges: #33342, #33517, #33531, #33532, #33538, #33541, #33544, #33552, #33554, #33555, #33560, #33563, #33565, #33566, #33572
- Failed merges: #33578
a27ed51

Manishearth added a commit to Manishearth/rust that referenced this pull request May 12, 2016

@Manishearth
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
Verified
This commit was signed with a verified signature.
@Manishearth Manishearth Manish Goregaokar
GPG key ID: 3BBF4D3E2EF79F98 Learn about signing commits
b4e4e84

bors added a commit that referenced this pull request May 12, 2016

@bors
Auto merge of #33582 - Manishearth:rollup, r=Manishearth 
Rollup of 11 pull requests

- Successful merges: #33450, #33531, #33532, #33541, #33544, #33552, #33554, #33555, #33560, #33563, #33566
- Failed merges:
e8fcf1b
@nagisa

This comment has been minimized.

Sign in to view
Copy link
Contributor

nagisa commented May 12, 2016

This is probably the cause for http://buildbot.rust-lang.org/builders/auto-win-gnu-64-nopt-t/builds/4202/steps/test/logs/stdio.

(rollup #33589)
@sfackler
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 #21889
Loading status checks…
9393e52

@sfackler sfackler force-pushed the sfackler:no-current-exe branch from 9c2e691 to 9393e52 May 12, 2016

@sfackler

This comment has been minimized.

Sign in to view
Copy link
Member Author

sfackler commented May 12, 2016

Thanks, fixed.

@bors r=alexcrichton
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented May 12, 2016

📌 Commit 9393e52 has been approved by alexcrichton

bors added a commit that referenced this pull request May 12, 2016

@bors
Auto merge of #33589 - eddyb:rollup, r=eddyb 
Rollup of 15 pull requests

- Successful merges: #33342, #33517, #33531, #33532, #33538, #33541, #33544, #33552, #33554, #33555, #33560, #33563, #33565, #33566, #33572
- Failed merges: #33578
302c9ac

eddyb added a commit to eddyb/rust that referenced this pull request May 13, 2016

@eddyb
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
bab9fa2

@sfackler sfackler added the beta-nominated label May 14, 2016

Manishearth added a commit to Manishearth/rust that referenced this pull request May 14, 2016

@Manishearth
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
4a1ce98

@Manishearth Manishearth referenced this pull request May 14, 2016

Merged

Rollup of 9 pull requests #33632

bors added a commit that referenced this pull request May 14, 2016

@bors
Auto merge of #33632 - Manishearth:rollup, r=Manishearth 
Rollup of 9 pull requests

- Successful merges: #33544, #33552, #33554, #33555, #33560, #33566, #33572, #33574, #33576
- Failed merges:
6ba8a1a

@bors bors merged commit 9393e52 into rust-lang:master May 14, 2016
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sfackler sfackler deleted the sfackler:no-current-exe branch May 15, 2016

@alexcrichton alexcrichton added the beta-accepted label May 16, 2016

@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member

alexcrichton commented May 17, 2016

Accepted for backport at libs triage

aturon added a commit that referenced this pull request May 17, 2016

@aturon
Merge pull request #33693 from alexcrichton/beta-next 
Backport #33554
3369382

@alexcrichton alexcrichton removed the beta-nominated label May 18, 2016

@retep998 retep998 referenced this pull request May 31, 2016

Closed

Rust stack backtrace filled with unknown on Windows 8.1 #33985

src/libstd/sys/common/gnu/libbacktrace.rs
// 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

This comment has been minimized.

Sign in to view
Copy link
@hoodie

hoodie Jul 18, 2016

Contributor

is this meant to say: "down what executable this is. ..." ?

This comment has been minimized.

Sign in to view
Copy link
@sfackler

sfackler Jul 18, 2016

Author Member

Oops, yeah.

@petrochenkov petrochenkov referenced this pull request Oct 25, 2016

Closed

Fix backtraces on Windows/GNU #37359

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.