Originally reported to the Drupal security team by
@Nux →
on 28 March 2022 (#176602), which was subsequently considered a case for security hardening and not a vulnerability.
---
Initially I though there was an RCE vulnerability... But after reviewing code I cannot find it. Long story short -- I think you should still consider locking simpletest and other test folders.
You can see this vulnerability by:
1. Install the module / Drupal.
2. As a user with no permissions do POST requests.
3. ...(attached requests; filtered are those that come through)
My analysis
After reviewing the code of the simpletest module I'm almost sure they were shooting blanks.
The attackers where sending quite a lot of POST request to PHP files in `/modules/simpletest`. Most of the requests failed with 500. For example when you call this:
```
/modules/simpletest/tests/upgrade/drupal-7.trigger.database.php
```
You will get an error because of undefined `db_insert`. So this is mostly causing PHP errors spamming your logs.
Maybe they were trying to find some files that actually execute something on their own. I can imagine some dev changing tests to execute stuff
The only thing I could find that might actually do something is `http.php` and `https.php` scripts. But the check of UA in `drupal_valid_test_ua` seems solid enough. Attackers did change their UA, but not in a way that would allow running tests. I guess if `str_replace` would have some weird bugs then an attacker might brake something in the `$_SERVER` array, but that is a stretch.
So yes, I was not able to find anything to cause immediate threats. But maybe you would see something different in those requests.
Locking tests
Event though I haven't found immediate problems, I still think it would still be better to make sure the files cannot be executed at all. For simpletest I think there is no need for it be accessible from a remote machine. At least not by default.
One solution would be to create a htaccess with `Require local` like the one attached. Quick and easy. Should work for 99% of cases.
Another solution would be to define some constant in a main file and do quick death in includes. So something like that in all files that are not meant to be executed directly:
```
if (!defined('NO_HACKS')) { http_response_code(403); exit; }
```
But I guess that would be too much work at this point and probably an overkill.
PS: I was not able to capture payloads of the attackers. They used POST and at that time I didn't have that in the logs (I'm now logging values in POST requests so will let you know if I get more info).
Problem/Motivation
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Contributors
- Nux
- boombatower
- DamienMcKenna
- Lendude
- mcdruid
- catch
Coordination:
- cilefen