astropy / astropy/astroquery

Some astroquery modules can lead to a Remote Code Execution vulnerability

Open
#2,777 14 comments 0 reactions 1 assignee Claimed by @ceb8 View on GitHub
bug cache
Dominant language
Python
Stars
791
Forks
451
Avg merge
1d 3h
Merged PRs (30d)
4

Description

In case you want to try to find and exploit the vulnerability yourself see the CTF challenge before reading rest of this ticket: https://hack.cert.pl/challenge/astrology
The application to downloads some files with ALMA astroquery module, and it's enough to give attacker a Remote Code Execution under certain conditions.

Warning: spoilers ahead!

The exploitation chain described here is specific to ALMA astroquery module, however few other modules contain similar code and might also be vulnerable. ALMA was picked because it provides a simple way to fulfil some of the attack scenario prerequisites.

Stage 1
The attack requires convincing user to use attacker-controller URL. This is especially interesting for ALMA since there are multiple mirrors and it's not suspicious to see different valid URLs. There is no validation of the `dataarchive_url` to make sure it's a legitimate one, nor there is any kind of certificate-pinning to avoid MITM, so it is possible to use astroquery ALMA module with attacker-controlled URL.

Stage 2
Once astroquery is connected to attacker-controller URL all requests will go there, including TAP queries and file download requests.
The vulnerability is located in the download_files function:

```python
try:
filename = re.search("filename=(.*)",
check_filename.headers['Content-Disposition']).groups()[0]
except KeyError:
log.info(f"Unable to find filename for {file_link} "
"(missing Content-Disposition in header). "
"Skipping to next file.")
continue

if savedir is not None:
filename = os.path.join(savedir,
filename)
```
Notice that it's implicitly assumed that `filename` is really just a filename, but if this header contains a relative path with some `../` components, then the result of `os.path.join` can jump outside of `savedir` and point to any path.
This means that downloading a file from attacker-controlled URL can lead to `arbitrary file write`.

Stage 3
Arbitrary file write in itself can lead to data loss, but without root privileges can't be immediately escalated into code execution.
Fortunately (or not) astroquery can help us with that. There is a feature of astroquery, enabled by default (at least for ALMA), which allows to turn arbitrary file write into remote code execution -> `cache=True`
Astroquery caches HEAD requests into a `pickle` files in predictable locations. Pickle is not simple serialized data format (like json or xml) but rather a bytecode for a simple stack-based virtual machine. Loading a pickle allows to execute any python code we want.
We can use the `arbitrary file write` from the content-disposition parsing bug to create (or overwrite) a pickle for some specific file with malicious content, and once user attempts to download this file the pickle will be loaded and code executed.
This step requires attacker to know the astroquery cache directory location, which by default requires knowing the `username` to find the home dir location.

Suggestions:
1. It might be worth to consider at least some verification of the URLs or even certificate pinning.
2. Parsing Content-Disposition header clearly can be tricky, so it might be better to use some standard (and bug-free) implementation, instead of having each module have a its own.
3. Pickle is a very dangerous choice for serialization mechanism and should be avoided unless absolutely necessary, and I'm not sure if this should be enabled by default at all.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.