clowd / Clowd.Squirrel

Quick and easy installer and automatic updates for cross-platform dotnet applications
426 stars 39 forks source link

DLL Hijacking #103

Closed simader closed 2 years ago

simader commented 2 years ago

Hello, Does anybody have an idea how we could prevent DLL Hijacking in WPF application? Could we enable somehow hash checks? I am asking, because you already need to somehow check the hashes to be able to get the updates dlls during Releasify. Thanks for any hint. Best, Andreas

peppy commented 2 years ago

I think I've mentioned something similar to this as being an eventual requirement for us (wanting to check digital signatures on all files in a new update before applying it) so would be interested to hear if there's any existing events which can be hooked to do this, or any best practice recommendations.

caesay commented 2 years ago

There's not currently a good way to do this, as Squirrel does not ship a list of files or hashes in a release. Provided Squirrel shipped a list of files in a release (and their hashes), and the packages themselves were signed to prevent tampering, this could work. I'll leave this issue open for now as an enhancement request.

caesay commented 2 years ago

Just some more comments after further consideration.

We do not currently verify the checksum of each individual file in a package (except when we are applying a delta, to check for corruption). However, the remote RELEASES file contains a checksum of each entire nupkg, and this is verified when downloading the package. Assuming that the remote server can be trusted, this means the requirement of:

wanting to check digital signatures on all files in a new update before applying it

Is already solved, more or less, because the entire package has already been validated. Signing the package could be an optional added protection from MITM, but will need to put a lot of thought into how we allow you to rotate signing keys. I discussed this some in #4.

If the above is not satisfactory, and you wanted to add additional checks before an update is actually applied, this is actually already possible using the standard UpdateManager API (eg. check, download, apply, are separate steps you can execute independently).

So really that leaves us with two questions

I think the answer to both is no, primarily because it feels a bit niche and adds significant complexity, but keen to hear other thoughts.

peppy commented 2 years ago

Assuming that the remote server can be trusted

This was my main reason for doing digital signature verification (relying on code signing root certificate verification), actually. I had first hand experience of this being a problem with one service I ran in the past, and made changes as a result to absolutely ensure that even in the case of a server takeover, it would not allow code to be deployed to a user's machine.

In the case of using github releases as a distribution means, risks are still present if user permissions are setup too liberally, or if an admin level user has their account compromised (allowing uploading a foreign releases as if it is official, which would still see the nuget package checksums matching).

but will need to put a lot of thought into how we allow you to rotate signing keys

This is the hardest part of implementation, agree.

If the above is not satisfactory, and you wanted to add additional checks before an update is actually applied, this is actually already possible using the standard UpdateManager API (eg. check, download, apply, are separate steps you can execute independently).

This sounds like it might be an ample solution if you see implementing a solution at the squirrel level being unwanted. In the interest of spreading awareness of the issue, I'd argue it would be a nice addition / best practice for an updater library, but can understand if it's out of scope.

  • Is individual file hash validation required after updates, checked during launch to protect against it being modified by the user.

I think this particularly is out of scope for squirrel (unless it was just a helper method which could be reused in user code). Not all applications would care about this and it's entering a bit of a different territory.

caesay commented 2 years ago

I'd argue it would be a nice addition / best practice for an updater library

I agree that package signing and verification can (should?) be part of Squirrel's responsibilities. It would close off the concerns here and the ones in #4.

I would be willing to work on this, however I don't have enough experience with signing in general to feel comfortable doing this without some guidance and discussion.

caesay commented 2 years ago

I am closing this in favour of continued discussion on #110