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

fix(macOS): Tauri V2 Update Permission Denied Error #2067

Open
wants to merge 7 commits into
base: v2
Choose a base branch
from

Conversation

jLynx
Copy link
Contributor

@jLynx jLynx commented Nov 18, 2024

Description

Improves the macOS app update process by adding proper handling for permission-denied scenarios and implementing a more robust extraction mechanism.
This is the Tauri V2 version of my previous PR tauri-apps/tauri#10427

image

Changes

  • Creates separate temporary directories for backup and extraction to better manage the update process
  • Adds proper directory creation for extracted files to prevent path-related issues
  • Implements fallback to AppleScript with admin privileges when standard file operations fail due to permissions
  • Improves error handling and cleanup in case of failures during the update process
  • Maintains atomic update operations to prevent partial updates

Why

The current implementation could fail on macOS when the app lacks sufficient permissions to modify its own directory, which is common in certain installation locations. This update provides a more reliable solution by:

  1. Properly handling permission-denied scenarios
  2. Using admin privileges when necessary
  3. Ensuring clean rollback in case of failures
  4. Addressed the problem of Tauri updater failing for standard (non-admin) users on macOS with a "Permission Denied" error as stated in this issue [bug] [macOs] Tauri updater fails with "Tauri API error: Permission Denied (os error 13)" for standard (non-admin) users tauri#8104.

Testing

  • Tested app updates on an admin account and a use account
  • Verified successful updates both with and without admin privileges

@jLynx jLynx requested a review from a team as a code owner November 18, 2024 00:54
@jLynx
Copy link
Contributor Author

jLynx commented Nov 27, 2024

@tweidinger In response to your previous comment on the V1 version of this. I understand the concerns about TOCTU attacks, but I believe the current implementation has several security mitigations that make it reasonably safe:

  1. Controlled Temporary Directory:
  • We're using tempfile which creates directories with restricted permissions (700 by default)
  • The temporary directories are created with random names and are only accessible to the current user
  • The lifetime of the temporary files is very short - only during the update process
  1. Limited Attack Window:
  • The window of vulnerability is extremely small - only during the file move operation
  • The privileged operation (via osascript) is only used for the final move, not for extraction or execution
  • We're not executing any content from the temporary directory
  1. Existing Validations:
  • The update package is still verified via signature before extraction
  • The contents are validated in memory before being written to the temp directory
  • The final move operation is atomic

The current approach solves real-world issues with updates on macOS while maintaining a reasonable security posture. The attack surface requires an attacker to already have user-level access and the ability to time their attack precisely during the update process.

CC: @lucasfernog @chippers

@tweidinger
Copy link
Contributor

tweidinger commented Dec 18, 2024

Hey @jLynx thanks for the v2 port and your comprehensive summary ❤️

While I disagree with that the "Limited Attack Window" (as this is a completely normal TOCTU scenario where timing is part of the attack) and the "Existing Validations" in any way fully prevent the elevation attack from a non privileged process running as the same user with the proposed changes, I see the real world use case and agree that there are more than enough easier ways to get privilege escalation from an unprivileged process.

I am going to add documentation to the threat model of the updater in a subsequent PR to explain the TOCTU risk and scenarios a bit more and see this as motivation to re-design the updater plugin where validation is possible at download, extraction, move and rollback without breaking non-admin/admin installs and the installer binaries.

Otherwise as discussed with @lucasfernog I would approve this PR from a security perspective and after he tests it locally and also approves the code changes we would merge this.

Would require a change file as well to be able to be merged as this changes behavior on MacOS (in a non breaking way).

Copy link
Contributor

github-actions bot commented Dec 18, 2024

Package Changes Through 2fb4f54

No changes.

Add a change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

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.

2 participants