golang / vulndb

[mirror] The Go Vulnerability Database
Other
562 stars 60 forks source link

x/vulndb: potential Go vuln in github.com/getgrav/grav: GHSA-9436-3gmp-4f53 #1949

Closed GoVulnBot closed 1 year ago

GoVulnBot commented 1 year ago

In GitHub Security Advisory GHSA-9436-3gmp-4f53, there is a vulnerability in the following Go packages or modules:

Unit Fixed Vulnerable Ranges
github.com/getgrav/grav 1.7.42.2 <= 1.7.42.1

Cross references: No existing reports found with this module or alias.

See doc/triage.md for instructions on how to triage this report.

modules:
    - module: github.com/getgrav/grav
      versions:
        - introduced: TODO (earliest fixed "1.7.42.2", vuln range "<= 1.7.42.1")
      vulnerable_at: 0.0.0-20230718184108-490bdd6ce7aa
      packages:
        - package: github.com/getgrav/grav
summary: grav Server-side Template Injection (SSTI) mitigation bypass
description: |-
    ### Summary The fix for SSTI using `|map`, `|filter` and `|reduce` twigs
    implemented in the commit
    [71bbed1](https://github.com/getgrav/grav/commit/71bbed12f950de8335006d7f91112263d8504f1b)
    introduces bypass of the denylist due to incorrect return value from
    `isDangerousFunction()`, which allows to execute the payload prepending double
    backslash (`\\`)

    ### Details The `isDangerousFunction()` check in version 1.7.42 and onwards
    retuns `false` value instead of `true` when the `\` symbol is found in the
    `$name`.

    ```php ... if (strpos($name, "\\") !== false) { return false; }

    if (in_array($name, $commandExecutionFunctions)) { return true; } ... ``` Based
    on the code where the function is used, it is expected that any dangerous
    condition would return `true` ```php /**
    * @param Environment $env
    * @param array $array
    * @param callable|string $arrow
    * @return array|CallbackFilterIterator
    * @throws RuntimeError */ function mapFunc(Environment $env, $array, $arrow) {
    if (!$arrow instanceof \Closure && !is_string($arrow) ||
    Utils::isDangerousFunction($arrow)) { throw new RuntimeError('Twig |map("' .
    $arrow . '") is not allowed.'); } ``` when `|map('\system')` is used in the
    malicious payload, the single backslash is dropped prior to reaching
    `strpos($name, '\\')` check, thus `$name` variable already has no backslash, and
    the command is blacklisted because it reaches the `if (in_array($name,
    $commandExecutionFunctions)) {` validation step.

    However if `|map('\\system')` is used (i.e. double backslash), then the
    `strpos($name, "\\") !== false` takes effect, and `isDangerousFunction()`
    returns `false` , in which case the `RuntimeError` is not generated, and
    blacklist is bypassed leading to code execution.

    ### Exploit Conditions This vulnerability can be exploited if the attacker has
    access to:

    1. an Administrator account, or
    2. a non-administrator, user account that has Admin panel access and
    Create/Update page permissions

    ### Steps to reproduce

    1. Log in to Grav Admin using an administrator account.
    2. Navigate to `Accounts > Add`, and ensure that the following permissions are
    assigned when creating a new low-privileged user:
    - Login to Admin - Allowed
    - Page Update - Allowed
    3. Log out of Grav Admin
    4. Login using the account created in step 2.
    5. Choose `Pages -> Home`
    6. Click the `Advanced` tab and select the checkbox beside `Twig` to ensure that
    Twig processing is enabled for the modified webpage.
    7. Under the `Content` tab, insert the following payload within the editor:
    ```{{ ['id'] | map('\\system') | join() }}```
    8. Click the `Preview` button. Observe that the output of the id shell command
    is returned in the preview.

    ### Mitigation

    ```diff diff --git a/system/src/Grav/Common/Utils.php
    b/system/src/Grav/Common/Utils.php index 2f121bbe3..7b267cd0f 100644 ---
    a/system/src/Grav/Common/Utils.php +++ b/system/src/Grav/Common/Utils.php @@
    -2069,7 +2069,7 @@ abstract class Utils }

    if (strpos($name, "\\") !== false) {
    - return false;
    + return true; }

    if (in_array($name, $commandExecutionFunctions)) {

cves:

gopherbot commented 1 year ago

Change https://go.dev/cl/514636 mentions this issue: data/excluded: batch add 31 excluded reports