From 7e06a81bbd09100e50273a235c38fa8853dae5c9 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sun, 2 Feb 2025 00:59:08 +0100
Subject: [PATCH] Fix fallback paths in fast_long_{add,sub}_function

This was asked to be checked in https://github.com/php/php-src/pull/17472#issuecomment-2591325036

There are 2 issues:
1) The UB in the if can overflow, and can be fixed by using zend_ulong
   for the sum/sub.
2) fast_long_sub_function() has a problem when result aliases.
   This is fixed in the same way as fast_long_add_function() works.

Closes GH-17666.
---
 NEWS                  |  1 +
 Zend/zend_operators.h | 18 ++++++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index 49ecf9f3de0..963a20eedce 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,7 @@ PHP                                                                        NEWS
     compilation). (ilutov)
   . Fixed bug GH-17618 (UnhandledMatchError does not take
     zend.exception_ignore_args=1 into account). (timwolla)
+  . Fix fallback paths in fast_long_{add,sub}_function. (nielsdos)
 
 - Opcache:
   . Fixed bug GH-17654 (Multiple classes using same trait causes function
diff --git a/Zend/zend_operators.h b/Zend/zend_operators.h
index fd1db6f4063..76e95a92d41 100644
--- a/Zend/zend_operators.h
+++ b/Zend/zend_operators.h
@@ -722,11 +722,13 @@ overflow: ZEND_ATTRIBUTE_COLD_LABEL
 	 * have read the values of op1 and op2.
 	 */
 
+	zend_long sum = (zend_long) ((zend_ulong) Z_LVAL_P(op1) + (zend_ulong) Z_LVAL_P(op2));
+
 	if (UNEXPECTED((Z_LVAL_P(op1) & LONG_SIGN_MASK) == (Z_LVAL_P(op2) & LONG_SIGN_MASK)
-		&& (Z_LVAL_P(op1) & LONG_SIGN_MASK) != ((Z_LVAL_P(op1) + Z_LVAL_P(op2)) & LONG_SIGN_MASK))) {
+		&& (Z_LVAL_P(op1) & LONG_SIGN_MASK) != (sum & LONG_SIGN_MASK))) {
 		ZVAL_DOUBLE(result, (double) Z_LVAL_P(op1) + (double) Z_LVAL_P(op2));
 	} else {
-		ZVAL_LONG(result, Z_LVAL_P(op1) + Z_LVAL_P(op2));
+		ZVAL_LONG(result, sum);
 	}
 #endif
 }
@@ -804,11 +806,19 @@ overflow: ZEND_ATTRIBUTE_COLD_LABEL
 		ZVAL_LONG(result, llresult);
 	}
 #else
-	ZVAL_LONG(result, Z_LVAL_P(op1) - Z_LVAL_P(op2));
+	/*
+	 * 'result' may alias with op1 or op2, so we need to
+	 * ensure that 'result' is not updated until after we
+	 * have read the values of op1 and op2.
+	 */
+
+	zend_long sub = (zend_long) ((zend_ulong) Z_LVAL_P(op1) - (zend_ulong) Z_LVAL_P(op2));
 
 	if (UNEXPECTED((Z_LVAL_P(op1) & LONG_SIGN_MASK) != (Z_LVAL_P(op2) & LONG_SIGN_MASK)
-		&& (Z_LVAL_P(op1) & LONG_SIGN_MASK) != (Z_LVAL_P(result) & LONG_SIGN_MASK))) {
+		&& (Z_LVAL_P(op1) & LONG_SIGN_MASK) != (sub & LONG_SIGN_MASK))) {
 		ZVAL_DOUBLE(result, (double) Z_LVAL_P(op1) - (double) Z_LVAL_P(op2));
+	} else {
+		ZVAL_LONG(result, sub);
 	}
 #endif
 }