Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.PhpTransliteration::readLanguageOverrides
Since $langcode comes from the url it is quite easy to include ANY php file on the server. Even if it situated outside Drupal root.
The simplest way to reproduce the bug is to navigate to following path: machine_name/transliterate?text=а&langcode=../../../../../../index
The vulnerability is mitigated by the fact that since PHP 5.3.4 paths with NULL in them are considered as invalid. Otherwise the attacker could include any files on a server, not just php scripts.
However opportunity to include any php files may be used to run external php applications that might be situated outside Drupal root or protected by http authorization. For instance, I was able to get access to my database by inclusion adminer.php.
originally reported to Drupal 8 bug bounty
https://tracker.bugcrowd.com/submissions/df2aa05dfef8649c33e119bc27e22f4...
reporter: Chi https://www.drupal.org/user/556138
Triaged in private by the Drupal security team - does not affect Drupal 6 or 7
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | test_routes.txt | 774 bytes | chx |
| #25 | routes.txt | 11.79 KB | chx |
| #19 | interdiff.txt | 672 bytes | chx |
| #19 | 2508654_19.patch | 3.6 KB | chx |
| #12 | 2508654_11-test-only.patch | 1.93 KB | chx |
Comments
Comment #1
tim.plunkettComment #2
chx CreditAttribution: chx commentedOh damn.
Comment #3
chx CreditAttribution: chx commentedDid you know that all sorts of parenthesis are a valid delimiter for regular expressions?
Compare http://3v4l.org/LEjg3 to http://3v4l.org/iMf6O
Comment #4
Pere OrgaI confirm that patch in #3 fixes the issue.
Comment #5
dawehnerSeriously, we should better have a test for it
Comment #6
Dave ReidComment #7
Gábor HojtsyComment #8
PolAs soon as this is fixed, I'll include it in service_container too. (ref: https://github.com/LionsAd/service_container/pull/67)
Comment #9
dawehnerWhich is IMHO just overengineering and makes it harder to start with as a beginner ...
Comment #10
chx CreditAttribution: chx commentedThis is not easy to test.
Comment #11
chx CreditAttribution: chx commentedSimplified (a little) and explained the test.
Comment #12
chx CreditAttribution: chx commentedWith more @covers metadata.
Comment #13
dawehnerThank you chx!
Comment #17
chx CreditAttribution: chx commentedExpected fail.
Comment #18
alexpottNice find and nice test.
I've grepped the codebase looking for similar code and couldn't find any. I think we need to document on TransliterationInterface that the langcode is user input and should be treated with suspicion. Just in case another writes a replacement for the transliteration service.
Comment #19
chx CreditAttribution: chx commentedSure. I am not reposting the test-only patch since there's no change to that.
Comment #20
dawehnerCool
Comment #21
alexpottCommitted 337f182 and pushed to 8.0.x. Thanks!
Comment #23
Gábor HojtsyYay, yayay!
Comment #24
pwolanin CreditAttribution: pwolanin as a volunteer and at Acquia commentedComment #25
chx CreditAttribution: chx commentedI talked to Ivan about how he discovered this and with his permission I am posting it here:
That was quite interesting.
I was trying to find unprotected urls on the D8 site which might disclose some private information. So that I exported all available Drupal paths to the text file using
\Drupal::service('router.route_provider')and then created a simple php script that read the file and sent http requests to the site using curl library.The path machine_name/transliterate took my attention because it was one of the few urls accessible by anonymous user (see script output in attachment). So I decided to check what is inside the controller that is responsible for that route.
Having found preg_replace that takes parameters directly from request object I was able to make Drupal to emit warnings by passing wrong values to that variables through url. But nothing else because replacement pattern was wrapped with '@' and null byte injection did not work on my local environments. So I decided to postpone this and track
$langcodevariable that was not used directly insideMachineNameController::transliterate(). It leads me toPhpTransliteration::readLanguageOverrides()where I found the file inclusion vulnerability .Actually I did not spot that wrong regular expression pattern that was supposed to prevent file inclusion. I just put wrong value for $langcode variable to url and make yourself sure that it works. :)
Later when I was submitting initial issue about invalidated parameters for
preg_replace#2508735: Code injection via preg_replace() I found that null byte injections works on some PHP 5.4 versions that Drupal 8 was still supporting at that moment. So that issue also became security.