swiftlang / swift-package-manager

The Package Manager for the Swift Programming Language
Apache License 2.0
9.72k stars 1.34k forks source link

[SR-13346] Swift Package Manager Security Risk #4513

Open swift-ci opened 4 years ago

swift-ci commented 4 years ago
Previous ID SR-13346
Radar rdar://problem/66586184
Original Reporter kanecheshire (JIRA User)
Type Bug
Environment Should note that while I do have Xcode 12 betas installed, Xcode on the machine where I've reproduced this is installed from the App Store and xcode-select is using that version, not Xcode 12.
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 2 | |Component/s | Package Manager | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: cea434d0fea89ad03bb6f84afcc6d96f

Issue Description:

Sorry if this isn't the correct place to report this but there's no issue tracker on the SPM repo and after writing a post about this issue someone pointed me to this Jira instance to report it here.

I've found an easy way for anyone to execute arbitrary code via Swift Package Manager in the Package.swift file:

let package: Package = {
  doSomething()
  return Package(...)
 }()
func doSomething() {
// Run any code in here as soon as the package is resolved, including anything from AppKit like NSWorkspace, or Foundation like FileManager, Process or URLSession.
}

The above code compiles and resolves correctly as a Package.swift file, and if hosted remotely somewhere like GitHub, the doSomething() function will be called as soon as it's resolved. Currently, that means in Xcode 11 it will call doSomething() silently before you've even finished adding the dependency since Xcode clones the repo and resolves the package before adding it to a target.

Example repo: https://github.com/KaneCheshire/maleficent To reproduce:

git clone https://github.com/KaneCheshire/maleficent
cd maleficent
swift build
# note that `say` will speak a greeting to you by name

I have no idea the best way to make this safer at a SPM level, but since it provides a way for people to silently execute code at the same privilege level that Xcode has (since it's running inside an Xcode process) I wanted to raise the issue asap.

swift-ci commented 4 years ago

Comment by Kane Cheshire (JIRA)

In hindsight this was also possible to do just by creating a function to return a string as the name of the package and execute the function there. But the concern of security is still there, this enables anyone to silently execute code as soon as a package is resolved from a remote source, which can be changed at any time. Cocoapods has a similar function in the podspec but it logs warnings when it happens and also gets advertised as a feature so people are aware of it (plus you have to explicitly declare it in the podspec)

swift-ci commented 4 years ago

Comment by Kane Cheshire (JIRA)

Some further comments on this, since I've been playing around with it (I'm not a pen tester so forgive my naivety). I can easily get this to search all certificates and create identities to use private keys. Although it does ask the user to confirm, I also managed to get it to show a standard "macOS wants to make some changes" dialog while trying to use private keys to sign some arbitrary data.

I can also easily create a file enumerator starting at "/", which will iterate over the entire filesystem. If it hits a protected directory like Photos, the system shows an alert asking the user to allow Xcode permission to access it (not at all visible that it's actually the package process causing the popup).

These are really rudimentary tests because I'm a noob but it feels like this stuff is easy to manipulate and trick people into allowing, aside from the fact that you can just access any other filesystem or execute any Process command you want.

swift-ci commented 4 years ago

Comment by Jeremy W. Sherman (JIRA)

it looks like this might be addressed with the v5.3 switch to compiled, rather than interpreted, manifests. the sandbox exceptions will then no longer be applied: https://github.com/apple/swift-package-manager/blob/13d60ca2e2a53c4f5b7998c650f62828d3726f6b/Sources/PackageLoading/ManifestLoader.swift#L822-L837

swift-ci commented 4 years ago

Comment by Kane Cheshire (JIRA)

Oh awesome!

neonichu commented 4 years ago

@swift-ci create

neonichu commented 4 years ago

This is a 5.3 regression, in https://github.com/apple/swift-package-manager/commit/1a4e3f23a3857dbad629f8e68680ca0030a6bbad#diff-fcff114931afa1ba8ce17a7e9b0c379c we moved some code around which ended up putting `sandbox-exec` at the end of the commandline instead of the start, so it doesn’t actually do anything.

PR: https://github.com/apple/swift-package-manager/pull/2848

thereaderfortyfive commented 2 years ago

Is this still a security issue / vulnerability and that is why it is open?

I found it here (https://medium.com/@KaneCheshire/swift-package-manager-is-a-security-risk-4d13f3a7bc3b)... it makes me hesitant to use SwiftPM...

benjiwolff commented 7 months ago

I made a small "malicious" package to test this bug. It seems that it does not manage to start any processes when resolved, fetched or when a SwiftPM executable or unsandboxed Xcode macOS App is run. If I understand correctly, the Package.swift itself is executed in a separate sandbox?

Print statements are shown as warnings when the package is installed using SwiftPM, however I see no such warning in Xcode.

image