"PHP" / Говнокод #11570 Ссылка на оригинал

0

  1. 1
  2. 2
  3. 3
  4. 4
  5. 5
  6. 6
  7. 7
  8. 8
  9. 9
  10. 10
  11. 11
  12. 12
  13. 13
  14. 14
  15. 15
$hash = md5($eshopId."::".
                        $orderId."::".
                	    $_REQUEST["serviceName"]."::".
                        $_REQUEST["eshopAccount"]."::".
                        $_REQUEST["recipientAmount"]."::".
            			$_REQUEST["recipientCurrency"]."::".
            			$_REQUEST["paymentStatus"]."::".
            			$_REQUEST["userName"]."::".
                        $_REQUEST["userEmail"]."::".
            			$_REQUEST["paymentData"]."::".
            			$secretKey);
            if (strtoupper($_REQUEST["hash"]) != strtoupper($hash)) {
                die('Err: wrong hash.');
            }
......

я конечно уважаю modx но ето через чур
http://bezumkin.ru/modx/minishop/extra/intellectmoney.html

Запостил: Sulik78 Sulik78, (Updated )

Комментарии (11) RSS

  • ava bormand # Ссылка на оригинал 0

    А что не так, кроме отступов?

    P.S. Разве что на implode("::", array(..тут все поля...)) заменить.
    Ответить

    • ava bormand # Ссылка на оригинал 0 ⇈

      P.P.S. Предложите, пожалуйста, свой вариант этого кода.
      Ответить

      • ava Sulik78 # Ссылка на оригинал 0 ⇈

        логика тупа. зачем вообще такой тупой хэш?
        ну или хотя бы уже
        implode('::', $_REQUEST)
        Ответить

        • ava dev6alexander # Ссылка на оригинал 0 ⇈

          Не пойдет если по мимо стандартной информации о проплате они пришлют вам еще кастомные поля к примеру ID пользователя проплатившего заказ. Тогда ваша подпись будет не верна.
          Ответить

        • ava Elvenfighter # Ссылка на оригинал 0 ⇈

          А в PHP есть что-то типа hash-slice? Например в Perl это бы звучало приблизительно как:
          join('::', @_REQUEST{serviceName, eshopAccount, recipientAmount, recipienCurrency, paymentStatus, userName, userEmail, paymentData}, $secretKey);
          Ответить

        • ava bormand # Ссылка на оригинал 0 ⇈

          > Зачем вообще такой тупой хэш.
          Ну авторы платежной системы указали в документации, что считать его нужно именно так. И обсуждать "почему именно так" бессмысленно. Это требование платежной системы. Да и по опыту работы с одной из них скажу - та же хрень, только в профиль, поля немного другие.

          > implode('::', $_REQUEST)
          Ни в коем случае. В хеше важен порядок, и некоторые поля (да тот же $secretKey) в реквесте не размещены.
          Ответить

          • ava bormand # Ссылка на оригинал 0 ⇈

            Комментарии Капитана Очевидность:

            Данная схема - типичный MAC (http://en.wikipedia.org/wiki/Message_authentication_code) основанный на хэш функции. $secretKey известен только данному серверу и серверу платежной системы. Злоумышленник не знает $secretKey, поэтому он не сможет сформировать поддельное сообщение.

            P.S. Смысла упрощать данный код не вижу. В данном состоянии (если причесать отступы, и сделать implode("::", array(...)) вместо конкатенации) он вполне читаем и верифицируем.
            Ответить

        • ava Lowezar # Ссылка на оригинал 0 ⇈

          Вы понимаете, что сделав просто implode('::', $_REQUEST) вы не гарантируете
          1. порядок полей;
          2. даже если лишних параметров не пришлют - сюда же попадёт присланный $_REQUEST['hash'], с которым, собственно, сравнить надо;
          3. не видно, какие поля должны использоваться.

          ...Я мог, наверное, промолчать и дать разобраться "зачем?" самому, но у нас как раз на днях тоже так "оптимизировали" не подумав. За день до релиза.
          Работает - не трожь. ©
          Ответить

          • ava Sulik78 # Ссылка на оригинал 0 ⇈

            я написал абстрактно. и не думал что мое сообщение примут буквально.
            имел ввиду сформированный "ручками" массив вместо $_REQUEST.
            Ответить

      • ava Sulik78 # Ссылка на оригинал 0 ⇈

        ну если уже и придется делать "тупой" хэш
        $parsed = array_map(function($key) {
        return isset($_REQUEST[$key]) ? $_REQUEST[$key] : $key;
        }, array(
                 $eshopId, $orderId, 'serviceName', 'eshopAccount', 'recipientAmount',
                   'recipienCurrency', 'paymentStatus', 'userName', 'userEmail',
                   'paymentData', $secretKey
                   ));
    
    var_dump(implode('::', $parsed));

        ну а раз уж modx то вообще надо в массив обернуть функцию и засунуть туда класс.
        return $this->getOption($key, $_REQUEST, $key);
        Ответить

        • ava bormand # Ссылка на оригинал 0 ⇈

          Прием попахивает (в данном случае он, конечно, будет работать, но в других может и поломаться) - если одна из переменных (к примеру $secretKey) будет равна чему-то, что есть в $_REQUEST, результат будет неправильным.

          Плюс ко всему - этот вариант "поедает" предупреждения интерпретатора - в оригинальном коде, если бы мы опечатались или платежники убрали поле, в лог бы выдалось предупреждение, здесь же мы предупреждения не получим, и код молча прохеширует имя поля вместо его значения.
          Ответить

Добавить комментарий

Где здесь C++, guest?!

    А не использовать ли нам bbcode?

    ↑
    8
    ↓