From 19ddc627786162b553f8cfceff247e2fea68d1ca Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 25 Mar 2023 19:06:07 +0100 Subject: [PATCH 1/2] Fix undefined behaviour when writing 32-bit values in phar/tar.c As shown on the CI runs on my fork (which runs with UBSAN), the pointers can sometimes be unaligned when trying to write. This is UB and on platforms like ARM this *can* result in a bus error. Replace it with memcpy, which at least on x86 and powerpc architectures does result in the same assembly code. Closes GH-10940. --- ext/phar/tar.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ext/phar/tar.c b/ext/phar/tar.c index 99b6b9812de..80a17d69318 100644 --- a/ext/phar/tar.c +++ b/ext/phar/tar.c @@ -1244,13 +1244,15 @@ nostub: return EOF; } #ifdef WORDS_BIGENDIAN -# define PHAR_SET_32(var, buffer) \ - *(uint32_t *)(var) = (((((unsigned char*)&(buffer))[3]) << 24) \ - | ((((unsigned char*)&(buffer))[2]) << 16) \ - | ((((unsigned char*)&(buffer))[1]) << 8) \ - | (((unsigned char*)&(buffer))[0])) +# define PHAR_SET_32(destination, source) do { \ + uint32_t swapped = (((((unsigned char*)&(source))[3]) << 24) \ + | ((((unsigned char*)&(source))[2]) << 16) \ + | ((((unsigned char*)&(source))[1]) << 8) \ + | (((unsigned char*)&(source))[0])); \ + memcpy(destination, &swapped, 4); \ + } while (0); #else -# define PHAR_SET_32(var, buffer) *(uint32_t *)(var) = (uint32_t) (buffer) +# define PHAR_SET_32(destination, source) memcpy(destination, &source, 4) #endif PHAR_SET_32(sigbuf, phar->sig_flags); PHAR_SET_32(sigbuf + 4, signature_length); From 6f56c00498c56abebec56cc8db76307cc640387f Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 25 Mar 2023 23:10:27 +0100 Subject: [PATCH 2/2] Fix undefined behaviour in GENERATE_SEED() Signed multiply overflow is undefined behaviour. If you run the CI tests with UBSAN enabled on a 32-bit platform, this is quite easy to hit. On 64-bit it's more difficult to hit though, but not impossible. Closes GH-10942. --- ext/standard/php_rand.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/standard/php_rand.h b/ext/standard/php_rand.h index e8018722772..ada63d47f5c 100644 --- a/ext/standard/php_rand.h +++ b/ext/standard/php_rand.h @@ -61,9 +61,9 @@ (__n) = (__min) + (zend_long) ((double) ( (double) (__max) - (__min) + 1.0) * ((__n) / ((__tmax) + 1.0))) #ifdef PHP_WIN32 -#define GENERATE_SEED() (((zend_long) (time(0) * GetCurrentProcessId())) ^ ((zend_long) (1000000.0 * php_combined_lcg()))) +#define GENERATE_SEED() (((zend_long) ((zend_ulong) time(NULL) * (zend_ulong) GetCurrentProcessId())) ^ ((zend_long) (1000000.0 * php_combined_lcg()))) #else -#define GENERATE_SEED() (((zend_long) (time(0) * getpid())) ^ ((zend_long) (1000000.0 * php_combined_lcg()))) +#define GENERATE_SEED() (((zend_long) ((zend_ulong) time(NULL) * (zend_ulong) getpid())) ^ ((zend_long) (1000000.0 * php_combined_lcg()))) #endif PHPAPI void php_srand(zend_long seed);