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

Possible Overwrite of dot files when Updating AppImage from Home Directory #160

Open
antony-jr opened this issue Oct 29, 2020 · 17 comments
Open
Labels

Comments

@antony-jr
Copy link

antony-jr commented Oct 29, 2020

This is just a Hypothesis, no experiments so far. POC is available -> https://youtu.be/PbNthAK2WKQ

Let's say you have a AppImage without GPG signs(not so uncommon) and it points to some url. Let's say a domain or Github. Let us assume that the attacker has found a way to somehow edit the update channels (DNS Spoofing?, or DNS hack so to redirect to his evil server?).

Now if he releases a new release but now he does the following,

  • Use the legacy zsync tool to generate a zsync file of a bash script.
  • Now edits the Filename entry to .bashrc
  • Now renames his bash script to .bashrc
  • Uploads all required files to Github or the release channel.

Now let us assume a AppImage is updated from the Home directory. AppImageUpdate would simply sync from the update channels and when every checksum is verified it will rename the file to the target file name as given in the zsync file. Now the updater is pointing to the home directory as the output directory.

The important thing to note here is that AppImageUpdate moves the old file i.e moves the original .bashrc file in home to .bashrc.zs-old and renames the temporary file(which is a bash script) to .bashrc.

It is worthy to note that the Qt version of the updater does not do this. I've implemented this many years ago when the library was first created.

Does not touch the host system , like AppImageUpdate does(i.e AppImageUpdate sometimes deletes user files without any content integrity checks but this library does not touch the host machine at all , if the target file name exists it will rename the new target file with the current date time in ISO 8601 format placed in the middle of the filename , Thus avoiding deleting any files in the host machine.)

But if .bashrc does not exists then even my library is vulnerable to this exploit.

Now the attacker has exploited the user and can launch the attack once the user opens the terminal or just logs in? i.e whenever the .bashrc is executed

Some users of linux can find this but not the layman.

If you say you can check if the downloaded file is a AppImage, I've spoofed the update tool to think a bash script is AppImage and still run as .bashrc.

# Warning don't execute this in home directory
dd if=appimagetool.AppImage of=.bashrc iflag=count_bytes count=11

Now simply write your bash code after 10 or so lines I guess, I wrote it right after the binary stuff and it still works.

Please correct me if I'm wrong and please verify if this hypothesis can be implemented. You can say that if the server gets hacked then the AppImage itself can be made into an exploit but if it runs from Firejail even with a simple profile then this exploit is useless. But the attacker can gain some sort of advantage by doing the hack this way since AppImageUpdate is not run inside the Firejail generally. I think the default settings of AppImageLauncher does store the AppImages away from Home directory which makes this exploit useless here.

But assume if the developer itself is evil then he/she builds some sort of trust and release first 10 releases clean without anything suspicious and one release he decides to exploit everyone. So does this intentionally. Again to note, if a AppImage is run from Firejail simple attacks like this can be mitigated, but this type of vulnerability allows the attacker to somehow bypass this because of a flaw in the AppImageUpdate

CC: @TheAssassin
EDIT: See POC -> https://youtu.be/PbNthAK2WKQ

@antony-jr
Copy link
Author

Please feel free to close this anytime.

@probonopd
Copy link
Member

Thanks @antony-jr for this consideration.

Please feel free to close this anytime.

Has it been proven to be wrong?

@antony-jr
Copy link
Author

antony-jr commented Oct 29, 2020

Has it been proven to be wrong?

Nope. But I do not have a POC.

EDIT: I was just hinting you that you can close this issue if you have other important work and think that this is not viable.

@antony-jr
Copy link
Author

@probonopd Actually this exploit works pretty good, See the POC -> https://youtu.be/PbNthAK2WKQ

@antony-jr
Copy link
Author

If you want to test my updater, It won't even update this type of thing because of a small bug but anyways even if it does, it renames the file. (Tested with the fix to bug, it updates as AppImageUpdate but at final stage it renames to revised).

@probonopd probonopd added the bug label Oct 29, 2020
@antony-jr
Copy link
Author

antony-jr commented Oct 29, 2020

So in a way, This exploit or hack allows an attacker to gain full access to the computer with just AppImageUpdate updating a untrusted AppImage which runs in Firejail or QEMU or Virtual Machine but still can hack because of a bug in AppImageUpdate or zsync2 if AppImageUpdate is not run from Firejail or QEMU or Virtual Machine.

So what this implies is that AppImageUpdate itself has to be run inside Firejail or QEMU or Virtual Machine along with the untrusted AppImage. I think Firejail itself is weak here if access to the real home directory is possible.

I never tried other location based attacks, like using ../filename hacks but if it is possible then this is something very bad.

@TheAssassin
Copy link
Member

Well, next time, please use responsible disclosure for such discussions.

I'll have a look at this potential exploit. I could well imagine that the filenames need to be sanitized for the same reason as in appiamged/libappimage, too. But I'll have to have a look myself.

@antony-jr
Copy link
Author

antony-jr commented Oct 29, 2020

Well, next time, please use responsible disclosure for such discussions.

I'll have a look at this potential exploit. I could well imagine that the filenames need to be sanitized for the same reason as in appiamged/libappimage, too. But I'll have to have a look myself.

For testing you can use the releases on this repo -> https://github.com/antony-jr/AppImageUpdate-Exploit-001-POC

I don't have any way to contact the core developers(like you) of AppImage via secure channels like email, there is IRC but I can't really write in detail there. It would be nice to put up some official email in the official site and also your GPG Public key to encrypt my message.

I don't want you to rush this and please take your time. Thank you for your time.

@TheAssassin
Copy link
Member

IRC is already significantly more private, as there's something like private/direct messages.

But publishing an official contact address is a good idea.

@antony-jr
Copy link
Author

Even if the AppImage was signed I was able to execute this exploit. So even GPG signs won't save you here. So only way to know if a AppImage is tampered or not is to use the provided GPG signature file or digest file by the author. Trying to map the AppImage into memory will crash your program if the given file is not a valid ELF binary. So it would be good to check the current AppImage GPG signature and digest first before update itself to prevent attacks like this, Because crash before the update is better than after moving the files.

@antony-jr
Copy link
Author

antony-jr commented Oct 30, 2020

EDIT: Thanks to https://github.com/AppImage/zsync2/blob/10e85c0eab4da4a1a85a949fb0f69d77cef9972f/src/zsclient.cpp#L406 this is not possible. For now.

I can infer from https://github.com/AppImage/zsync2/blob/10e85c0eab4da4a1a85a949fb0f69d77cef9972f/src/zsclient.cpp#L874 and https://github.com/AppImage/zsync2/blob/10e85c0eab4da4a1a85a949fb0f69d77cef9972f/lib/libzsync/zsync.c#L209

That if the Filename entry is something like /usr/bin/firefox can actually replace firefox with a evil version. That too an AppImage because it can run without anything. Also the user has no idea where this stuff is saved. Makes this exploit more dangerous than before. Further more, the executable bit is set default by the so called "Desktop Integrations"

But thankfully the official AppImageUpdate fails with permission error. Fortunately. But if the user were to run with sudo then there is no saving.

@antony-jr antony-jr changed the title Possible exploit when updated from Home Directory? Remote Code Execution when Updating untrusted AppImage Oct 30, 2020
@antony-jr antony-jr changed the title Remote Code Execution when Updating untrusted AppImage Possible Remote Code Execution when Updating untrusted AppImage Oct 30, 2020
@antony-jr
Copy link
Author

antony-jr commented Oct 30, 2020

EDIT: I think the update fails if filename has path component, which is nice so this is only Home directory exploit.
So it does not care if the AppImage is home directory, we just do ~/.bashrc in the filename entry of zsync file. (Not tested though)

Thanks to https://github.com/AppImage/zsync2/blob/10e85c0eab4da4a1a85a949fb0f69d77cef9972f/src/zsclient.cpp#L406

@antony-jr antony-jr changed the title Possible Remote Code Execution when Updating untrusted AppImage Possible Remote Code Execution when Updating untrusted AppImage from Home Directory Oct 30, 2020
antony-jr added a commit to antony-jr/QAppImageUpdate that referenced this issue Oct 30, 2020
@TheAssassin
Copy link
Member

Remote Code Execution

That is just the wrong term. An RCE vulnerability is usually characterized by allowing an adversary to execute arbitrary code through some sort of existing interface. A good example is having a PHP script on some random webserver where code can be injected through, e.g., some query string argument, so it's run by that webserver.

What you describe is a classic virus infection.


Let's start by analyzing a few of your assumptions.

Let us assume that the attacker has found a way to somehow edit the update channels (DNS Spoofing?, or DNS hack so to redirect to his evil server?).

Both options are rather unlike to work, unless you don't employ HTTPS. cURL validates the certificates, and as long as you don't manage to get a valid certificate which the system would accept, you'd still have to "hack" the original server. (This is, usually, also easier than trying to do DNS spoofing of any kind).

AppImageUpdate would simply sync from the update channels and when every checksum is verified it will rename the file to the target file name as given in the zsync file. Now the updater is pointing to the home directory as the output directory.

That's true to some extent. AppImageUpdate will, unless you let it overwrite the original file, use the filename listed in the .zsync file. Let's call it a problem, and discuss it later on. (I don't think it's really a security problem in the sense you think, though).

Does not touch the host system , like AppImageUpdate does(i.e AppImageUpdate sometimes deletes user files without any content integrity checks but this library does not touch the host machine at all , if the target file name exists it will rename the new target file with the current date time in ISO 8601 format placed in the middle of the filename , Thus avoiding deleting any files in the host machine.)

I would call this a lack of functionality rather than a "fix". The idea behind AppImageUpdate is, intentionally, to use the remote filename. This is in line with earlier incarnations and with the underlying zsync tool.

But if .bashrc does not exists then even my library is vulnerable to this exploit.

There's other files you could overwrite, e.g., .profile, ... Depends on the system you want to infect. It's not limited to this specific filename.

But I don't think the part where .bashrc could be overwritten is a big problem. The moment you update an application and the update mechanism has been hijacked by an adversary, you're in trouble already.

I figure your intention to bring up .bashrc as an example is that this way, the user doesn't actively have to run the application in order to execute the adversary's code. Of course there's no guarantee a regularly updated AppImage with malicious code would be run. But realistically, how are the chances that the attacked person would run the updated AppImage? I think they're pretty high.

Being able to overwrite .bashrc is not a big addition in such a scenario. In fact, it may be counterproductive. The best virus infection is one that isn't easy to discover. .bashrc is probably one of the most obvious places for a virus. Even if I went for that, I'd install my virus somewhere else ASAP and would immediately restore the original situation to cover my traces. Also, the name .bashrc will show up in the logging.

In my eyes, the major issue is that the attacker needs to have found a way to infect the file to update from in the first place. And that's something that's beyond the scope of these tools anyway.

Please correct me if I'm wrong and please verify if this hypothesis can be implemented.

It can. But I would say, the fact you can write to .bashrc doesn't increase severity in any way.

but if it runs from Firejail even with a simple profile then this exploit is useless

Perhaps. But firejail isn't such a well built solution for sandboxing AppImages anyway. Without having trustworthy(!) application-specific profiles (e.g., created by the user upon first run by selecting the capabilities an application should have, ideally assisted by suggestions from a config within the AppImage (Android-like), see TheAssassin/AppImageLauncher#99 for more information) you'll end up limiting applications too much or you're too generous.

I think the default settings of AppImageLauncher does store the AppImages away from Home directory which makes this exploit useless here.

Not necessarily. I've had an idea, based on the appimaged/libappimage exploit we've seen recently. Have you tried updating to ../.bashrc yet...?
I don't have time, but someone should test this. In case this works, we need to sanitize the filenames better, as it is done in libappimage now.

I've checked your later comments, indeed zsync2 checks whether a path prefix is contained. So, the approach is, rather than sanitizing a filename, we reject malicious ones. Fair enough.
Even when using HTTP (like many Mirrorbrain instances do), we've by the way got some mitigation in place. IIRC we implemented content digest support. So, as long as the main mirror is served via HTTPS, we have a digest that can be validated.
One thing that should be noted is that AppImageUpdate should warn users if HTTP is used and we don't have content digests. That's something that is missing so far.

But assume if the developer itself is evil then he/she builds some sort of trust and release first 10 releases clean without anything suspicious and one release he decides to exploit everyone. So does this intentionally.

To be fair, the assumption behind AppImage is that they allow application authors to provide their apps directly to the user, and if you don't trust them, you should not run them. It's as simple as that. If a developer turns out to be evil, well, you're always screwed. Check the controversy about Nano Adblocker (first link in English language that turned up: https://www.ghacks.net/2020/10/16/time-to-remove-nano-adblocker-and-defender-from-your-browsers-except-firefox/).


Let's talk about the attack model for a minute. Your adversary, at first, is described as a third party that manages to hijack the update channel. Doesn't matter if they hack the server, manage to break into GitHub, spoof DNS or whatever else.

Such an attack would be effectively mitigated by secure signatures. I'm highlighting the term secure because often, signing of AppImages is done insecurely, e.g., by putting key material on third party services or generally computers connected to the Internet. There's a reason that, for instance, F-Droid, an alternative app store for Android which only ships free/open-source software, uses an air gap computer to store the key material, and someone does it manually. Sure, it slows down the process, but it's worth it. Try steal the key material, it won't be easy. Plus, you, being F-Droid, do not have to trust any third party. Storing key material anywhere but infra you control is bad.

Following your second suggestion, let's assume the adversary is the original application author. In that case, this entire attack model you describe is way more complex than needed. If I were to infect one of my applications, I'd nest that code somewhere deep inside so it can't easily be found. There's no need to overwrite .bashrc. As mentioned before, I don't consider this to be the main issue.


So, does a secure signature effectively mitigate the attack?

Well, I think the answer is yes. Once an AppImage is signed, the remote file which is being updated needs to carry a valid signature with the same key. That's how AppImageUpdate has been designed to work. This is a form of "transitive trust", and has been described in some lengthy issue in detail.

Even if you'd manage to download a new AppImage to .bashrc, AppImageUpdate will, unless it crashes, remove that file again, as it is not signed correctly. As an attacker, you'd have to also sign your .bashrc. And this is pretty much impossible. The signature algorithm relies on the fact that the file is an ELF binary with a specific section. You can't simulate that in a .bashrc. So, you'd better be off shipping an actual AppImage anyway.


So, in my opinion, the core issue is we allow the remote side to choose the filename of the file we create locally. The fact that you can overwrite stuff like .bashrc in $HOME doesn't make this specifically a vulnerability. You'd still have to hijack the update channel first.

The reason we allow this is to allow the remote side to use filenames that contain the version number for example. That adds a lot of value to the user, especially if they want to have multiple versions around in parallel. Our mantra is to allow users to work with plain files as the most basic UX. AppImageLauncher etc. are just tools built around this, they don't prevent users from using AppImages in this basic way.

As we've seen above, the attacker would have to first hijack the update channel. And once they can do this, without signatures, they can do any kind of infection. As said, the chances are not that low that a user will run an updated application, so there's no real need for the adversary to find a way to have the system autostart their code.

The attack vector is already limited to AppImages that already reside inside $HOME. zsync2 prohibits paths by disallowing / in those filenames. Unless you're inside $HOME (to overwrite .bashrc), or, e.g., .config/autostart/ to place a malicious desktop entry, ..., the attack doesn't work at all anyway.

The question is, how to proceed with letting the remote choose filenames? Well, we have a strong pro argument (convenience, UX) and some weak counter arguments. We already do some sort of validation (as it is always necessary when using values from a party you don't (want to) trust). The only thing we could do is remove the entire feature, but I'm not sure it's necessary.

For security-sensitive users, we could improve communication. For instance, we could highlight the output filename in the updater window outside the spoiler log, or, better, have the user confirm they want to use the filename provided by the remote end. I think we should go for the latter. A security sensitive user will be puzzled immediately if it says "Update found, new filename would be .bashrc".


Summary.

I don't think there's anything urgent to do in AppImageUpdate in this scenario. I think there's no severe security threat. As said, the term RCE is wrong per definition. I think you overrate the severity, too.

We have to educate users to make better use of signatures. We have to also stress that they should never share the private keys with anyone. Not evenwith Travis CI. Never share your key with anybody. Sure, that's manual work, but the gain is high. And, as @probonopd put it lately, if something can't be automated, make it so it's hard for you, not for your users.

A key takeaway is to always validate and/or sanitize your values. We've seen that the lack of sanitizing has led to a severe vulnerability in appimaged/libappimage. And it looks like @antony-jr's tool had a similar bug, too, until lately.

An actual TODO for AppImageUpdate is to write a little dialog the user has to confirm if we take the filename provided in the .zsync file. That should make sure one doesn't have trust the remote side completely. Instead, users who care have a simple and intuitive way to introspect this value, they don't have to have a look at the log (which could be too late; once you wrote to .bashrc you can't even open a terminal to fix the issue), and by making this introspection mandatory before writing to disk, it gives users an actual way to prevent such attacks rather than allowing them to fix it afterwards.

@antony-jr antony-jr changed the title Possible Remote Code Execution when Updating untrusted AppImage from Home Directory Possible Overwrite of dot files when Updating AppImage from Home Directory Nov 11, 2020
@antony-jr
Copy link
Author

So, in my opinion, the core issue is we allow the remote side to choose the filename of the file we create locally. The fact that you can overwrite stuff like .bashrc in $HOME doesn't make this specifically a vulnerability. You'd still have to hijack the update channel first.

@TheAssassin The point is, If the user runs an Untrusted AppImage from a VM or strict Firejail then the program have to do a lot of stuff to overwrite .bashrc or possibly execute dangerous code on the user host system. But with this "Bug"(?) AppImageUpdate itself can give a way for the attacker to gain access if the AppImage is unfortunately located in the Home Directory. Do you really think this is not bad?

Imagine this scenario, I downloaded a untrusted AppImage from somewhere. It recommends me to update the AppImage before use. So first to try it out, Because it's untrusted. I use QEMU + KVM(in snapshot mode) to run it safely.(The program attempts to write .bashrc to home and writes but since it's a QEMU snapshot it is not persistent and you mostly likely won't open a terminal from the QEMU instance of guest). Now I want to try the latest and greatest of the untrusted AppImage. I download AppImageUpdate and update the AppImage from Home Directory. That's it the attacker gains access.

Well, I think the answer is yes. Once an AppImage is signed, the remote file which is being updated needs to carry a valid signature with the same key. That's how AppImageUpdate has been designed to work. This is a form of "transitive trust", and has been described in some lengthy issue in detail.

Even if you'd manage to download a new AppImage to .bashrc, AppImageUpdate will, unless it crashes, remove that file again, as it is not signed correctly. As an attacker, you'd have to also sign your .bashrc. And this is pretty much impossible. The signature algorithm relies on the fact that the file is an ELF binary with a specific section. You can't simulate that in a .bashrc. So, you'd better be off shipping an actual AppImage anyway.

Not true at all. AppImageUpdate crashes if the AppImage is signed(and the new file is not a proper ELF binary because of a BUS error) and it does not remove the malicious .bashrc at all. I've tested this but did not write it in the original issue post. Try it for yourself, Edit any signed AppImage(with vbindiff) update information with the following update info and update it from home directory with AppImageUpdate.

gh-releases-zsync|antony-jr|AppImageUpdate-Exploit-001-POC|latest|ShareMyHost*-x86_64.AppImage.zsync

And trust me fixing a bus error is not easy. You just have to hope your program crashes before doing any damages to the host system.

And it looks like @antony-jr's tool had a similar bug, too, until lately.

Actually I had a weird bug which is that I used the filename entry of the zsync file to construct the url to the actual file. So if filename entry has .bashrc then it is url/.bashrc, since github does not allow this the update will fail. So the attacker has to ensure that the url is accessible with the dot for the update to continue. And in the latest version I just fixed that and just added some security fixes.

@antony-jr
Copy link
Author

antony-jr commented Nov 11, 2020

@TheAssassin If you think this is not serious you can put this in low priority or close this issue if wanted. I have no issue with this, I'm just reporting a possible attack which I think is bad in my opinion. 👍

@TheAssassin
Copy link
Member

TheAssassin commented Nov 11, 2020

Do you really think this is not bad?

I don't say it's not an issue, but I don't think it's a huge deal security wise. You also make the assumption that a) firejail was safe and the user would use it or b) someone would use a VM where they share the home directory or something. I don't think that's the general use case. Nor do I think sandboxing in general is a good concept for security. It can at most be used as a sort of "failsafe/last resort".

I downloaded a untrusted AppImage from somewhere.

This, being the underlying assumption, doesn't correlate with AppImage's trust model. If you don't trust an AppImage, do not use it. It's as simple as that.

As said, sandboxing itself should not serve as a protection for non-trustworthy code. Not even Docker makes big claims with regards to security. You must use a VM. Period.

QEMU + KVM(in snapshot mode)

Please name one generic user without an IT background of any kind who would do that. We don't seek to secure your edge case, we care about the general user.

You shouldn't use AppImageUpdate on non-trustworthy files on your main system in the first place. This is very similar to Wireshark, where the authors are aware that their application regularly has bugs and should not be used on malicious streams on a production system. In fact, they even recommend themselves to use something with a smaller codebase to capture the streams (e.g., tcpdump, not tshark), then move to a safe system (e.g., a VM without network access) and analyze the data there.

AppImageUpdate crashes if the AppImage is signed(and the new file is not a proper ELF binary because of a BUS error)

I have no clue what a "BUS error" is supposed to be. Please open a separate issue.

And trust me fixing a bus error is not easy. You just have to hope your program crashes before doing any damages to the host system.

How can you judge how difficult it is to fix a "bus error"?

You just have to hope your program crashes before doing any damages to the host system.

Well, that's always the hope in software engineering.

Edit: in no word you discuss my proposed system for mitigating the effects, which also the regular user could understand...

@antony-jr
Copy link
Author

Edit: in no word you discuss my proposed system for mitigating the effects, which also the regular user could understand...

I agree on making a small dialog before starting the update which shows the output file location and other stuff. Yes it can mitigate this if the user is not a complete linux novice.

I have no clue what a "BUS error" is supposed to be.

Bus error simply leads to a segmentation fault.

In our context. It happens when the CPU tries to read illegal memory location. When you have a file with ELF binary signature and AppImage signature you try to read data beyond that files size because you make an assumption that an ELF binary header should be some size as per the standard. Some binary like this will cause a segmentation fault in AppImageUpdate when even attempt to read it,

00000000  7f 45 4c 46 02 01 01 00  41 49 02 0a 0a 65 63 68  |.ELF....AI...ech|
00000010  6f 20 22 52 75 6e 6e 69  6e 67 20 73 6f 6d 65 20  |o "Running some |
00000020  63 6f 64 65 20 74 68 61  74 20 68 61 72 6d 73 20  |code that harms |
00000030  79 6f 75 21 22 0a 6c 73  0a 0a                    |you!".ls..|

How can you judge how difficult it is to fix a "bus error"?

Currently we map the AppImage(ELF Binary) and cast it to the ELF struct but you have to find a way to know if every entry of the elf binary you are mapping and casting is valid. So you have to do a very brief check before casting the mapped file which I think is tricky and difficult.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants