Skip to content

Refuse to launch the macOS agent from an unsafe bundle - #638

Open
SebTardif wants to merge 2 commits into
ivpn:developmentfrom
SebTardif:bugfix/helper-unsafe-bundle
Open

SebTardif wants to merge 2 commits into
ivpn:developmentfrom
SebTardif:bugfix/helper-unsafe-bundle

Conversation

@SebTardif

Copy link
Copy Markdown

PR type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other... Please describe:

PR checklist

  • I have read the CONTRIBUTING.md doc
  • The Git workflow follows our guidelines: CONTRIBUTING.md#git
  • I have added necessary documentation (if appropriate)

What is the current behavior?

If /Applications/IVPN.app is not owned by root, the privileged helper checks the signature of IVPN Agent only, then runs chown -R 0:0 on the whole app and launches the agent. Files in the bundle other than that binary are not part of the check.

is_safe_dir already requires both uid 0 and gid 0 (07bd5323). This PR does not change that test.

A private report of this was closed without a code change (GHSA-9vqc-pp5f-wmc9).

Issue number: N/A

What is the new behavior?

When the bundle is not safe, the helper logs the same message and exits. It does not change ownership and it does not launch the agent. check_signature is removed because that was its only caller.

Does this PR introduce a breaking change?

  • Yes
  • No

A bundle that is already root:wheel still launches. A bundle that is not root-owned no longer gets repaired by the helper. Install or repair has to fix ownership before the helper will start the agent.

Other information

Base is development.

If /Applications/IVPN.app is not root-owned, the helper checked
the agent binary signature and then chowned the whole bundle to
root before exec. Refuse to launch instead.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
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.

1 participant