SSRF vis role name on content translation screen

Created on 12 April 2024, about 1 year ago

Problem/Motivation

This was originally reported to the security team who decided it could be public
This issue was reported to IBM as a vulnerability in the IBM API Connect Developer Portal, however the actual vulnerability is in the underlying Drupal Core translation modules. The vulnerability allows one Drupal user with administrative permissions to create a page which can force another Drupal user to make a call to a server under the control of an attacker. I have tested this on a vanilla Drupal 10 site and the vulnerability is present there.

Steps to reproduce

1. Set up a HTTP server somewhere, this will be the target of the malicious request (I used "ncat -l -p 80 --keep-open" just to see the request come in, obviously an attacker would use a server that they control)

2. Enable the four Drupal Core modules Language, Interface Translation, Content Translation and Configuration Translation

3. Navigate to Manage->People->Roles and create a new role with a name of

">'><img src="http://127.0.0.1/evil">

4. Navigate to the roles list and from the action menu for our malicious role select "Translate"

5. Choose a language and press the add button

The browser will make a call to the malicious server -- in my example I can clearly see the request come in:

Proposed resolution

Change $value = '<span lang="' . $source_language->getId() . '">' . nl2br($source_config) . '</span>'; to render as plain text string with \Drupal\Component\Utility\Html::escape

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Active

Version

11.0 πŸ”₯

Component
Config translationΒ  β†’

Last updated 9 days ago

Created by

πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

Live updates comments and jobs are added and updated live.
  • Security improvements

    It makes Drupal less vulnerable to abuse or misuse. Note, this is the preferred tag, though the Security tag has a large body of issues tagged to it. Do NOT publicly disclose security vulnerabilities; contact the security team instead. Anyone (whether security team or not) can apply this tag to security improvements that do not directly present a vulnerability e.g. hardening an API to add filtering to reduce a common mistake in contributed modules.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @larowlan
  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10
  • πŸ‡ΊπŸ‡ΈUnited States akalata

    Updating steps to reproduce.

  • πŸ‡ΊπŸ‡ΈUnited States akalata
  • πŸ‡ΊπŸ‡ΈUnited States akalata

    Edit issue title

  • Pipeline finished with Failed
    4 months ago
    Total: 530s
    #417184
  • πŸ‡ΊπŸ‡ΈUnited States akalata
  • Pipeline finished with Success
    4 months ago
    Total: 1576s
    #417231
  • πŸ‡ΊπŸ‡ΈUnited States greggles Denver, Colorado, USA

    The title is SSRF (server side request forgery) which I understand to mean a request from the server to other machines on the server's network as described in portswigger and owasp.

    The description includes:

    The vulnerability allows one Drupal user with administrative permissions to create a page which can force another Drupal user to make a call to a server under the control of an attacker.

    and

    The browser will make a call to the malicious server

    That is not a server-side request.

    I can see how this would let an admin gain data about the site visitors, which is not ideal, but on most sites an admin who can enter translations can probably already make other changes to a site that would also gain information about a visitor.

    I'm surprised if this is the only place where a user with only the ability to translate could insert an image. I think other strings are similarly not filtered.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Seems the test-only failure is passing when would expect to fail so think that needs tweaking https://git.drupalcode.org/issue/drupal-3440399/-/jobs/4273224

  • Assigned to akalata
  • Status changed to Needs work 16 days ago
  • First commit to issue fork.
  • Pipeline finished with Failed
    16 days ago
    Total: 606s
    #500252
  • Pipeline finished with Failed
    16 days ago
    Total: 93s
    #500256
  • Pipeline finished with Success
    16 days ago
    Total: 352s
    #500257
  • πŸ‡«πŸ‡·France prudloff Lille

    I adjusted the test to fail without the fix.

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    this does a strip tags, that seems wrong and the escape then is pointless?

    if the source config has some HTML, you want to see that as the translation needs the same.

    It should just escape and should then also assert the escaped markup

  • Pipeline finished with Failed
    15 days ago
    Total: 456s
    #500862
  • Pipeline finished with Success
    15 days ago
    Total: 1238s
    #500888
  • πŸ‡«πŸ‡·France prudloff Lille

    I removed the strip_tags() call, I think it was added in order to not break ConfigTranslationUiModulesTest.
    And I added an assertion with the escaped string (it needs to be precise because other parts of the page already contain the escaped payload in other places).

Production build 0.71.5 2024