esotalk v1.0.0g4 /members/?search xss vulnerability

[evi1m0]

Vulnerability analysis

file: /var/www/esotalk/core/lib/functions.general.php

/**
 * Get a request input value, falling back to a default value if it is not set. POST will be searched first,
 * then GET, and then the fallback will be used.
 *
 * @param string $key The request input key.
 * @param mixed $default The fallback value.
 * @return mixed
 *
 * @package esoTalk
 */
function R($key, $default = "")
{
    if (!empty($_POST[$key])) return $_POST[$key];
    elseif (isset($_GET[$key])) return $_GET[$key];
    else return $default;
}

R func return, Find a use case:

file: /var/www/esotalk/core/controllers/ETMembersController.class.php

/**
 * Show the member list page.
 *
 * @param string $orderBy What to sort the members by.
 * @param mixed $start Where to start the results from. This can be:
 *      - An integer, in which case it will be used as a numerical offset.
 *      - pX, where X is the "page" number.
 *      - A letter to start from, if $orderBy is "name".
 * @return void
 */
public function action_index($orderBy = false, $start = 0)
{
    if (!$this->allowed("esoTalk.members.visibleToGuests")) return;

    // Begin constructing a query to fetch results.
    $sql = ET::SQL()->from("member m");

    // If we've limited results by a search string...
    if ($searchString = R("search")) {
    .........

$searchString = R("search")，payload here：

http://localhost/esotalk/index.php/members/?search=123'><svg/onload=alert(1)>

Fix

file: /var/www/esotalk/core/lib/functions.general.php

function R($key, $default = "")
{
    if (!empty($_POST[$key])) return htmlspecialchars($_POST[$key]);
    elseif (isset($_GET[$key])) return htmlspecialchars($_GET[$key]);
    else return $default;
}

[jgknight]

I will do some testing with this. The proposed fix looks good except I think we should use our sanitizeHTML function instead of calling PHPs htmlspecialchars function directly.

[jgknight]

Okay so the problem is coming in where that search field is being ran through sanitzeHTML, but it's still letting some characters get through that shouldn't be.

[evi1m0]

This is a very good :)