Skip to content

[Backtracing] Support redirection to a named file. #79007

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

Merged
merged 3 commits into from
Feb 14, 2025

Conversation

al45tair
Copy link
Contributor

Add the ability to specify a filename or directory name as the output-to setting in SWIFT_BACKTRACE. If the option is set to a directory name, generate a unique filename in that directory using the process name, process ID and timestamp.

rdar://136977833

Add the ability to specify a filename or directory name as the output-to
setting in `SWIFT_BACKTRACE`.  If the option is set to a directory name,
generate a unique filename in that directory using the process name,
process ID and timestamp.

rdar://136977833
@al45tair al45tair requested a review from mikeash as a code owner January 29, 2025 10:30
@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

Copy link
Member

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you! 👏

Copy link
Contributor

@mikeash mikeash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any security worries here with, say, a suid root binary being told to write to a file the user doesn't have permission to touch, or can we count on the standard OS permissions to take care of it?

@al45tair
Copy link
Contributor Author

al45tair commented Jan 29, 2025

Are there any security worries here with, say, a suid root binary being told to write to a file the user doesn't have permission to touch, or can we count on the standard OS permissions to take care of it?

The backtracer will never run for a setuid binary (and if you manage to force it somehow, I think there's even code in the swift-backtrace program to just terminate if it finds itself running as root). It also refuses to run if the process has various other kinds of privileges, so I think we're good on that front.

Mike rightly points out that it's possible for `open()` to fail
with `EINTR`, which we should handle here.

rdar://136977833
@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

We should harden the output path setting so that it can't be used
as part of an exploit to get a crashing process to overwrite a
file at an attacker-controller path, or to divert the crash report
to `/dev/null` to hide their tracks or other such undesirable
activity.

Take a copy of the setting at start-up and write-protect it to
prevent attackers overwriting it.

Note that we already protect against attempts to trigger the
backtracer from privileged programs (both on Darwin and Linux);
this is really a belt and braces measure to make life harder for
attackers.

rdar://136977833
@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

@al45tair al45tair merged commit f0050df into swiftlang:main Feb 14, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants