From f4c043a2ad3657ec7b8e5d93fcb15b704dd90309 Mon Sep 17 00:00:00 2001
From: Loz Calver <lozcalver@bigfork.co.uk>
Date: Mon, 16 May 2022 09:50:06 +0100
Subject: [PATCH] [CVE-2022-29254] Add extra validation on payment completion

---
 code/Service/AuthorizeService.php            | 13 +++++--------
 code/Service/CaptureService.php              | 20 +++++++++-----------
 code/Service/CreateCardService.php           | 13 +++++--------
 code/Service/NotificationCompleteService.php |  5 ++++-
 code/Service/PurchaseService.php             | 16 ++++++----------
 code/Service/RefundService.php               | 17 ++++++++---------
 code/Service/ServiceResponse.php             | 16 ++++++++++++++++
 code/Service/VoidService.php                 | 12 +++++-------
 8 files changed, 58 insertions(+), 54 deletions(-)

diff --git a/code/Service/AuthorizeService.php b/code/Service/AuthorizeService.php
index 220d418a..e9a4db5a 100644
--- a/code/Service/AuthorizeService.php
+++ b/code/Service/AuthorizeService.php
@@ -60,7 +60,7 @@ public function initiate($data = array())
             );
         } elseif ($serviceResponse->isError()) {
             $this->createMessage('AuthorizeError', $response);
-        } else {
+        } elseif ($serviceResponse->isSuccessful()) {
             $this->markCompleted('Authorized', $serviceResponse, $response);
         }
 
@@ -110,15 +110,12 @@ public function complete($data = array(), $isNotification = false)
 
         $serviceResponse = $this->wrapOmnipayResponse($response, $isNotification);
 
-        if ($serviceResponse->isError()) {
+        if ($serviceResponse->isAwaitingNotification()) {
+            Helper::safeExtend($this->payment, 'onAwaitingAuthorized', $serviceResponse);
+        } elseif ($serviceResponse->isError()) {
             $this->createMessage('CompleteAuthorizeError', $response);
-            return $serviceResponse;
-        }
-
-        if (!$serviceResponse->isAwaitingNotification()) {
+        } elseif ($serviceResponse->isSuccessful()) {
             $this->markCompleted('Authorized', $serviceResponse, $response);
-        } else {
-            Helper::safeExtend($this->payment, 'onAwaitingAuthorized', $serviceResponse);
         }
 
         return $serviceResponse;
diff --git a/code/Service/CaptureService.php b/code/Service/CaptureService.php
index a8a2bf24..80a5eab5 100644
--- a/code/Service/CaptureService.php
+++ b/code/Service/CaptureService.php
@@ -128,7 +128,9 @@ public function initiate($data = array())
 
         $serviceResponse = $this->wrapOmnipayResponse($response);
 
-        if ($serviceResponse->isAwaitingNotification()) {
+        if ($serviceResponse->isError()) {
+            $this->createMessage($this->errorMessageType, $response);
+        } elseif ($serviceResponse->isRedirect() || $serviceResponse->isAwaitingNotification()) {
             if ($diff < 0) {
                 $this->createPartialPayment(PaymentMath::multiply($amount, '-1'), $this->pendingState);
             } elseif ($diff > 0) {
@@ -136,17 +138,13 @@ public function initiate($data = array())
             }
             $this->payment->Status = $this->pendingState;
             $this->payment->write();
-        } else {
-            if ($serviceResponse->isError()) {
-                $this->createMessage($this->errorMessageType, $response);
-            } else {
-                if ($diff < 0) {
-                    $this->createPartialPayment(PaymentMath::multiply($amount, '-1'), $this->pendingState);
-                } elseif ($diff > 0) {
-                    $this->createPartialPayment($diff, $this->pendingState);
-                }
-                $this->markCompleted($this->endState, $serviceResponse, $response);
+        } elseif ($serviceResponse->isSuccessful()) {
+            if ($diff < 0) {
+                $this->createPartialPayment(PaymentMath::multiply($amount, '-1'), $this->pendingState);
+            } elseif ($diff > 0) {
+                $this->createPartialPayment($diff, $this->pendingState);
             }
+            $this->markCompleted($this->endState, $serviceResponse, $response);
         }
 
         return $serviceResponse;
diff --git a/code/Service/CreateCardService.php b/code/Service/CreateCardService.php
index 7f38d626..1bb871a2 100644
--- a/code/Service/CreateCardService.php
+++ b/code/Service/CreateCardService.php
@@ -60,7 +60,7 @@ public function initiate($data = array())
             );
         } elseif ($serviceResponse->isError()) {
             $this->createMessage('CreateCardError', $response);
-        } else {
+        } elseif ($serviceResponse->isSuccessful()) {
             $this->markCompleted('CardCreated', $serviceResponse, $response);
         }
 
@@ -110,15 +110,12 @@ public function complete($data = array(), $isNotification = false)
 
         $serviceResponse = $this->wrapOmnipayResponse($response, $isNotification);
 
-        if ($serviceResponse->isError()) {
+        if ($serviceResponse->isAwaitingNotification()) {
+            Helper::safeExtend($this->payment, 'onAwaitingCreateCard', $serviceResponse);
+        } elseif ($serviceResponse->isError()) {
             $this->createMessage('CompleteCreateCardError', $response);
-            return $serviceResponse;
-        }
-
-        if (!$serviceResponse->isAwaitingNotification()) {
+        } elseif ($serviceResponse->isSuccessful()) {
             $this->markCompleted('CardCreated', $serviceResponse, $response);
-        } else {
-            Helper::safeExtend($this->payment, 'onAwaitingCreateCard', $serviceResponse);
         }
 
         return $serviceResponse;
diff --git a/code/Service/NotificationCompleteService.php b/code/Service/NotificationCompleteService.php
index 6f8c7e07..5ca6981a 100644
--- a/code/Service/NotificationCompleteService.php
+++ b/code/Service/NotificationCompleteService.php
@@ -78,7 +78,10 @@ public function complete($data = array(), $isNotification = true)
         }
 
         // check if we're done
-        if (!$serviceResponse->isError() && !$serviceResponse->isAwaitingNotification()) {
+        if (!$serviceResponse->isError()
+            && !$serviceResponse->isAwaitingNotification()
+            && $serviceResponse->isSuccessful()
+        ) {
             $this->markCompleted($this->endState, $serviceResponse, $serviceResponse->getOmnipayResponse());
         }
 
diff --git a/code/Service/PurchaseService.php b/code/Service/PurchaseService.php
index 3e18bed6..7c75e6be 100644
--- a/code/Service/PurchaseService.php
+++ b/code/Service/PurchaseService.php
@@ -66,7 +66,7 @@ public function initiate($data = array())
             );
         } elseif ($serviceResponse->isError()) {
             $this->createMessage('PurchaseError', $response);
-        } else {
+        } elseif ($serviceResponse->isSuccessful()) {
             $this->markCompleted('Captured', $serviceResponse, $response);
         }
 
@@ -114,19 +114,15 @@ public function complete($data = array(), $isNotification = false)
         }
 
         $serviceResponse = $this->wrapOmnipayResponse($response, $isNotification);
-        if ($serviceResponse->isError()) {
-            $this->createMessage('CompletePurchaseError', $response);
-            return $serviceResponse;
-        }
 
-        // only update payment status if we're not waiting for a notification
-        if (!$serviceResponse->isAwaitingNotification()) {
-            $this->markCompleted('Captured', $serviceResponse, $response);
-        } else {
+        if ($serviceResponse->isAwaitingNotification()) {
             Helper::safeExtend($this->payment, 'onAwaitingCaptured', $serviceResponse);
+        } elseif ($serviceResponse->isError()) {
+            $this->createMessage('CompletePurchaseError', $response);
+        } elseif ($serviceResponse->isSuccessful()) {
+            $this->markCompleted('Captured', $serviceResponse, $response);
         }
 
-
         return $serviceResponse;
     }
 
diff --git a/code/Service/RefundService.php b/code/Service/RefundService.php
index c607325d..3c7686fe 100644
--- a/code/Service/RefundService.php
+++ b/code/Service/RefundService.php
@@ -121,21 +121,20 @@ public function initiate($data = array())
 
         $serviceResponse = $this->wrapOmnipayResponse($response);
 
-        if ($serviceResponse->isAwaitingNotification()) {
+        if ($serviceResponse->isError()) {
+            $this->createMessage($this->errorMessageType, $response);
+        } elseif ($serviceResponse->isRedirect() || $serviceResponse->isAwaitingNotification()) {
             if ($isPartial) {
                 $this->createPartialPayment(PaymentMath::multiply($amount, '-1'), $this->pendingState);
             }
             $this->payment->Status = $this->pendingState;
             $this->payment->write();
-        } else {
-            if ($serviceResponse->isError()) {
-                $this->createMessage($this->errorMessageType, $response);
-            } else {
-                if ($isPartial) {
-                    $this->createPartialPayment(PaymentMath::multiply($amount, '-1'), $this->pendingState);
-                }
-                $this->markCompleted($this->endState, $serviceResponse, $response);
+        } elseif ($serviceResponse->isSuccessful()) {
+            if ($isPartial) {
+                $this->createPartialPayment(PaymentMath::multiply($amount, '-1'), $this->pendingState);
             }
+
+            $this->markCompleted($this->endState, $serviceResponse, $response);
         }
 
         return $serviceResponse;
diff --git a/code/Service/ServiceResponse.php b/code/Service/ServiceResponse.php
index adde3a14..8f193058 100644
--- a/code/Service/ServiceResponse.php
+++ b/code/Service/ServiceResponse.php
@@ -94,6 +94,22 @@ public function getPayment()
         return $this->payment;
     }
 
+    /**
+     * Whether the response is marked as successful by Omnipay.
+     *
+     * @return bool
+     */
+    public function isSuccessful()
+    {
+        if ($this->omnipayResponse instanceof NotificationInterface) {
+            return $this->omnipayResponse->getTransactionStatus() === NotificationInterface::STATUS_COMPLETED;
+        } elseif ($this->omnipayResponse instanceof AbstractResponse) {
+            return $this->omnipayResponse->isSuccessful();
+        }
+
+        return false;
+    }
+
     /**
      * Whether or not this is an *offsite* redirect.
      * This is only the case when there's an Omnipay response present that *is* a redirect.
diff --git a/code/Service/VoidService.php b/code/Service/VoidService.php
index 1fde49a1..1c1c8361 100644
--- a/code/Service/VoidService.php
+++ b/code/Service/VoidService.php
@@ -88,15 +88,13 @@ public function initiate($data = array())
 
         $serviceResponse = $this->wrapOmnipayResponse($response);
 
-        if ($serviceResponse->isAwaitingNotification()) {
+        if ($serviceResponse->isError()) {
+            $this->createMessage($this->errorMessageType, $response);
+        } elseif ($serviceResponse->isRedirect() || $serviceResponse->isAwaitingNotification()) {
             $this->payment->Status = $this->pendingState;
             $this->payment->write();
-        } else {
-            if ($serviceResponse->isError()) {
-                $this->createMessage($this->errorMessageType, $response);
-            } else {
-                $this->markCompleted($this->endState, $serviceResponse, $response);
-            }
+        } elseif ($serviceResponse->isSuccessful()) {
+            $this->markCompleted($this->endState, $serviceResponse, $response);
         }
 
         return $serviceResponse;