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

Enable stack smashing protection #525

Open
jumde opened this issue Jul 11, 2018 · 5 comments
Open

Enable stack smashing protection #525

jumde opened this issue Jul 11, 2018 · 5 comments

Comments

@jumde
Copy link
Contributor

@jumde jumde commented Jul 11, 2018

  1. Automatic reference counting (-fobjc-arc) helps to prevent use-after-free and use-after-release bugs.

  2. Stack smashing protection (-fstack-protector-all) helps to prevent stack buffer overflows.

@bbondy bbondy added this to the Backlog milestone Jul 12, 2018
@garrettr
Copy link
Contributor

@garrettr garrettr commented Jul 12, 2018

We're already using -fstack-protector-strong for debug builds on macOS, probably worth auditing to make sure we're also using it in release builds and for all supported platforms.

@jumde
Copy link
Contributor Author

@jumde jumde commented Jul 19, 2018

Ignore Automatic reference counting.

@jumde jumde added this to Release channel blockers in Security & Privacy Jul 26, 2018
@jumde jumde changed the title Enable automatic reference counting and stack smashing protection Enable stack smashing protection Jul 30, 2018
@bbondy bbondy modified the milestones: 1.x Backlog, Releasable builds 0.55.x Sep 28, 2018
@bbondy
Copy link
Member

@bbondy bbondy commented Sep 28, 2018

Do you know the perf implication of turning these on @jumde ?
Based on What Garrett mentioned on Jul 12, do you know what's actually happening today?

@jumde jumde closed this Sep 28, 2018
@jumde jumde reopened this Sep 28, 2018
@bbondy bbondy added this to Security in 0.55.x - Release Sep 28, 2018
@jumde
Copy link
Contributor Author

@jumde jumde commented Sep 28, 2018

@bbondy

Looks like its enabled for the Helper processes but not the main binary

$ otool -I -v "Brave Browser Beta Helper" | grep stack
0x000000010000ee1c    29 ___stack_chk_fail
0x0000000100010008    30 ___stack_chk_guard
0x0000000100010100    29 ___stack_chk_fail
$ otool -I -v "Brave Browser Beta" | grep stack
@bbondy bbondy removed the audit-release label Sep 29, 2018
@bbondy bbondy modified the milestones: Releasable builds 0.55.x, 1.x Backlog Sep 29, 2018
@jumde jumde added the priority/P2 label Sep 29, 2018
@jumde
Copy link
Contributor Author

@jumde jumde commented Sep 29, 2018

Behavior is same in chrome.

@bbondy bbondy removed this from Release channel blockers in Security & Privacy Sep 29, 2018
@bbondy bbondy removed this from Security in 0.55.x - Release Sep 29, 2018
@jumde jumde added the sec-high label Sep 29, 2018
@rebron rebron modified the milestone: 1.x Backlog Feb 7, 2019
@rebron rebron modified the milestone: 1.x Backlog Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.