Analiza błędu w SDK PayU

Szybka analiza błędu błędu w SDK PayU o którym klienci zostali powiadomieni 4 sierpnia 2018 r.

Witaj, ze względu na bezpieczeństwo Twojego sklepu, zaktualizuj jak najszybciej do najnowszej wersji plugin, z którego obecnie korzystasz...

Takim dość groźnie brzmiącym mailem rozpoczęła się sobota dla wielu osób korzystających z bramek płatności PayU 🙂

Ale co tak naprawdę jest nie tak? Ostatnie zmiany można prześledzić w historii commit'ów na github'ie, poprawka jest w tym: https://github.com/PayU/plugin_prestashop/commit/846d5695a282fcd6e15c84a66d092dc023461245

Felerna funkcja to tools/sdk/OpenPayU/Util.php:99 (z przed zmiany):

public static function verifySignature($message, $signature, $signatureKey, $algorithm = 'MD5')
{
    $hash = '';

    if (isset($signature)) {
        if ($algorithm == 'MD5') {
            $hash = md5($message . $signatureKey);
        } else if (in_array($algorithm, array('SHA', 'SHA1', 'SHA-1'))) {
            $hash = sha1($message . $signatureKey);
        } else if (in_array($algorithm, array('SHA-256', 'SHA256', 'SHA_256'))) {
            $hash = hash('sha256', $message . $signatureKey);
        }

        if (strcmp($signature, $hash) == 0) {
            return true;
        }
    }

    return false;
}

 

Za co odpowiada i co jest z nią nie tak? Funkcja ta jak nazwa wskazuje weryfikuje poprawność sygnatury.

Sygnatura skrót MD5/SHA parametrów przesyłanych między API i aplikacją w celu potwierdzenia autentyczności nadawcy i odbiorcy a także danych wysyłanych w danym zapytaniu.

Jeśli funkcji podamy parametr $algorithm inny niż przewiduje metoda i $signature w postaci pustego stringa zwróci ona true gdyż nie wyliczy ona wtedy hash a tylko porówna zdefiniowany domyślnie pusty z pustą sygnaturą którą jej przykazaliśmy 🙂

$hash = '';

if (isset($signature)) {
    // Tu się nic nie dzieje

    if (strcmp($signature, $hash) == 0) {
        return true;
    }

 

Problemy w podanym kodzie to:

  1. Hash zdefiniowany domyślne jako pusty string
  2. Brak obsługi przypadkowego algorytmu
  3. Zamiast strcmp lepiej użyć ===
var_dump(strcmp('', false)); // int(0)
var_dump('' === false); // bool(false)

 

Błąd dość trywialny i trudno go przeoczyć ale jak wiemy pomyłki zdarzają się najlepszym. Pochwalić PayU należy za szybką reakcję i powiadomienie klientów o niebezpieczeństwie.

Docs:

 

Dodaj komentarz

Twój adres e-mail nie zostanie opublikowany. Wymagane pola są oznaczone *