isSuccessful() in ExpressCompletePurchaseResponse does not validate PAYMENTSTATUS === 'Completed'

[420-Bits]

The isSuccessful() method in ExpressCompletePurchaseResponse currently only checks for ACK being Success or SuccessWithWarning and that it's not a redirect:

public function isSuccessful()
{
    $success = isset($this->data['ACK']) && in_array($this->data['ACK'], array('Success', 'SuccessWithWarning'));
    return !$this->isRedirect() && $success;
}

However, PayPal can return an ACK=Success for transactions that are still in Pending, Voided, or Reversed state. This means isSuccessful() may return true even though the payment has not been completed.

⚠️ Security Impact
In a real-world integration using this library, a malicious actor could:

• Trigger completePurchase() with a valid token and PayerID
• Cause isSuccessful() to return true
• Mark a deposit or order as paid
• Without any funds actually being transferred

✅ Suggested Fix
Update the isSuccessful() method to include a check for:

isset($this->data['PAYMENTINFO_0_PAYMENTSTATUS']) && $this->data['PAYMENTINFO_0_PAYMENTSTATUS'] === 'Completed'

Full fix could look like:

public function isSuccessful()
{
    $ackSuccess = isset($this->data['ACK']) && in_array($this->data['ACK'], ['Success', 'SuccessWithWarning']);
    $paymentCompleted = isset($this->data['PAYMENTINFO_0_PAYMENTSTATUS']) && $this->data['PAYMENTINFO_0_PAYMENTSTATUS'] === 'Completed';

    return !$this->isRedirect() && $ackSuccess && $paymentCompleted;
}

💬 Notes

• This is a silent and critical vulnerability in the default flow.
• Many devs rely on isSuccessful() to determine whether to complete business logic (e.g., marking orders as paid).

Please consider patching this and tagging a security release.