razorpay / razorpay-php

Razorpay PHP Library
MIT License
181 stars 125 forks source link

Missing return statement in verifyPaymentSignature , verifyWebhookSignature and verifySignature methods in Utility class #377

Open Chunaravivek opened 1 month ago

Chunaravivek commented 1 month ago

Steps to reproduce the behavior

  1. Use the Razorpay PHP API with the Utility class.
  2. Call the verifyPaymentSignature or verifyWebhookSignature method.
  3. Missing return statement in verifySignature methods for $verifield value.

Expected behavior

The verifyPaymentSignature and verifyWebhookSignature methods should return the verification result (true or false) to indicate if the signature is valid.

Actual behavior

The verifyPaymentSignature and verifyWebhookSignature methods do not return any value, causing the verification result to be inaccessible.

Code snippets

namespace Razorpay\Api;

class Utility
{
    const SHA256 = 'sha256';

    public function verifyPaymentSignature($attributes)
    {
        $actualSignature = $attributes['razorpay_signature'];
        $paymentId = $attributes['razorpay_payment_id'];

        if (isset($attributes['razorpay_order_id']) === true)
        {
            $orderId = $attributes['razorpay_order_id'];
            $payload = $orderId . '|' . $paymentId;
        }
        else if (isset($attributes['razorpay_subscription_id']) === true)
        {
            $subscriptionId = $attributes['razorpay_subscription_id'];
            $payload = $paymentId . '|' . $subscriptionId;
        }
        else if (isset($attributes['razorpay_payment_link_id']) === true)
        {
            $paymentLinkId     = $attributes['razorpay_payment_link_id'];
            $paymentLinkRefId  = $attributes['razorpay_payment_link_reference_id'];
            $paymentLinkStatus = $attributes['razorpay_payment_link_status'];
            $payload = $paymentLinkId . '|'. $paymentLinkRefId . '|' . $paymentLinkStatus . '|' . $paymentId;
        }
        else
        {
            throw new Errors\SignatureVerificationError(
                'Either razorpay_order_id or razorpay_subscription_id or razorpay_payment_link_id must be present.');
        }

        $secret = Api::getSecret();
        return self::verifySignature($payload, $actualSignature, $secret);
    }

    public function verifyWebhookSignature($payload, $actualSignature, $secret)
    {
        return self::verifySignature($payload, $actualSignature, $secret);
    }

    public function verifySignature($payload, $actualSignature, $secret)
    {
        $expectedSignature = hash_hmac(self::SHA256, $payload, $secret);

        if (function_exists('hash_equals'))
        {
            $verified = hash_equals($expectedSignature, $actualSignature);
        }
        else
        {
            $verified = $this->hashEquals($expectedSignature, $actualSignature);
        }

        if ($verified === false)
        {
            throw new Errors\SignatureVerificationError('Invalid signature passed');
        }

        return $verified; // Missing in the original code
    }

    private function hashEquals($expectedSignature, $actualSignature)
    {
        if (strlen($expectedSignature) === strlen($actualSignature))
        {
            $res = $expectedSignature ^ $actualSignature;
            $return = 0;

            for ($i = strlen($res) - 1; $i >= 0; $i--)
            {
                $return |= ord($res[$i]);
            }

            return ($return === 0);
        }

        return false;
    }
}

Php version

PHP v8.2

Library version

razorpay-php v2.9.0

Additional Information

The methods verifyPaymentSignature and verifyWebhookSignature do not return the result of verifySignature which causes the caller to not know if the signature verification was successful. Adding return statements in these methods will resolve this issue.

Here is a possible fix for the methods:

public function verifyPaymentSignature($attributes)
{
    $actualSignature = $attributes['razorpay_signature'];
    $paymentId = $attributes['razorpay_payment_id'];

    if (isset($attributes['razorpay_order_id']) === true)
    {
        $orderId = $attributes['razorpay_order_id'];
        $payload = $orderId . '|' . $paymentId;
    }
    else if (isset($attributes['razorpay_subscription_id']) === true)
    {
        $subscriptionId = $attributes['razorpay_subscription_id'];
        $payload = $paymentId . '|' . $subscriptionId;
    }
    else if (isset($attributes['razorpay_payment_link_id']) === true)
    {
        $paymentLinkId     = $attributes['razorpay_payment_link_id'];
        $paymentLinkRefId  = $attributes['razorpay_payment_link_reference_id'];
        $paymentLinkStatus = $attributes['razorpay_payment_link_status'];
        $payload = $paymentLinkId . '|'. $paymentLinkRefId . '|' . $paymentLinkStatus . '|' . $paymentId;
    }
    else
    {
        throw new Errors\SignatureVerificationError(
            'Either razorpay_order_id or razorpay_subscription_id or razorpay_payment_link_id must be present.');
    }

    $secret = Api::getSecret();
    return self::verifySignature($payload, $actualSignature, $secret); // Add return statement
}

public function verifyWebhookSignature($payload, $actualSignature, $secret)
{
    return self::verifySignature($payload, $actualSignature, $secret); // Add return statement
}

public function verifySignature($payload, $actualSignature, $secret)
{
    $expectedSignature = hash_hmac(self::SHA256, $payload, $secret);

    if (function_exists('hash_equals'))
    {
        $verified = hash_equals($expectedSignature, $actualSignature);
    }
    else
    {
        $verified = $this->hashEquals($expectedSignature, $actualSignature);
    }

    if ($verified === false)
    {
        throw new Errors\SignatureVerificationError('Invalid signature passed');
    }

    return $verified; // Missing in the original code
}
captn3m0 commented 1 month ago

they throw exceptions.

Chunaravivek commented 1 month ago

they throw exceptions.

Yes but when if $verified has false matched (verifySignature method) , will be they throw exceptions. if $verified has true, How can know if verifySignature is true ? there get return verifySignature blank value so that's reason i have found issue. There missing return $verified last line if verifySignature has true.

$isSignatureValid = $this->verifyRazorpaySignature($razorpayOrderId, $razorpayPaymentId, $razorpay_signature);

if ($isSignatureValid) {} else {}

public function verifyRazorpaySignature($razorpayOrderId, $razorpayPaymentId, $razorpaySignature)
{
    return $this->utility->verifyPaymentSignature(array(
        'razorpay_order_id' => $razorpayOrderId,
        'razorpay_payment_id' => $razorpayPaymentId,
        'razorpay_signature' => $razorpaySignature
    ));
}