Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Chunaravivek opened this issue Jul 16, 2024 · 2 comments

Comments

@Chunaravivek
Copy link

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
Copy link
Contributor

they throw exceptions.

@Chunaravivek
Copy link
Author

Chunaravivek commented Jul 16, 2024

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
    ));
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants