From ad8a2754fa99ceaf40495185ad48191cf27559e2 Mon Sep 17 00:00:00 2001
From: Gleb Mazovetskiy <glex.spb@gmail.com>
Date: Tue, 5 Mar 2024 19:20:09 +0000
Subject: [PATCH] Fix some UB in movie_lib.cc (#117)

---
 src/movie_lib.cc | 81 ++++++++++++++++++++++++++++--------------------
 1 file changed, 47 insertions(+), 34 deletions(-)

diff --git a/src/movie_lib.cc b/src/movie_lib.cc
index 70123e6..fd2f158 100644
--- a/src/movie_lib.cc
+++ b/src/movie_lib.cc
@@ -5,6 +5,7 @@
 #include "movie_lib.h"
 
 #include <assert.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <string.h>
 
@@ -96,6 +97,10 @@ static int _MVE_sndDecompS16(unsigned short* a1, unsigned char* a2, int a3, int
 static void _nfPkConfig();
 static void _nfPkDecomp(unsigned char* buf, unsigned char* a2, int a3, int a4, int a5, int a6);
 
+static constexpr uint16_t loadUInt16LE(const uint8_t* b);
+static constexpr uint32_t loadUInt32LE(const uint8_t* b);
+static uint8_t getOffset(uint16_t v);
+
 // 0x51EBD8
 static int dword_51EBD8 = 0;
 
@@ -757,7 +762,7 @@ static unsigned char* _ioNextRecord()
         return NULL;
     }
 
-    _io_next_hdr = *(int*)(buf + (_io_next_hdr & 0xFFFF));
+    _io_next_hdr = loadUInt32LE(buf + (_io_next_hdr & 0xFFFF));
 
     return buf;
 }
@@ -854,7 +859,7 @@ int _MVE_rmStepMovie()
     }
 
     while (1) {
-        v5 = *(unsigned int*)((unsigned char*)v1 + v0);
+        v5 = loadUInt32LE((unsigned char*)v1 + v0);
         v1 = (unsigned short*)((unsigned char*)v1 + v0 + 4);
         v0 = v5 & 0xFFFF;
 
@@ -877,7 +882,7 @@ int _MVE_rmStepMovie()
             } else {
                 v7 = (v1[1] & 0x04) >> 2;
             }
-            v8 = *(unsigned int*)((unsigned char*)v1 + 6);
+            v8 = loadUInt32LE((unsigned char*)v1 + 6);
             if ((v5 >> 24) == 0) {
                 v8 &= 0xFFFF;
             }
@@ -1438,7 +1443,7 @@ static int _MVE_sndAdd(unsigned char* dest, unsigned char** src_ptr, int a3, int
     }
 
     if (a5) {
-        v12 = *(unsigned int*)src;
+        v12 = loadUInt32LE(src);
         src += 4;
 
         *(unsigned int*)dest = v12;
@@ -1868,8 +1873,6 @@ static void _nfPkDecomp(unsigned char* a1, unsigned char* a2, int a3, int a4, in
     int i;
     int j;
     ptrdiff_t v10;
-    int v11;
-    int v13;
     int byte;
     unsigned int value1;
     unsigned int value2;
@@ -1919,38 +1922,36 @@ static void _nfPkDecomp(unsigned char* a1, unsigned char* a2, int a3, int a4, in
                         break;
                     case 2:
                     case 3:
-                        byte = *a2++;
-                        v11 = word_51F618[byte];
-                        if (v7 == 3) {
-                            v11 = ((-(v11 & 0xFF)) & 0xFF) | ((-(v11 >> 8) & 0xFF) << 8);
-                        } else {
-                            v11 = v11;
+                        if (1) {
+                            byte = *a2++;
+                            uint16_t offset = word_51F618[byte];
+                            if (v7 == 3) {
+                                offset = ((-(offset & 0xFF)) & 0xFF) | ((-(offset >> 8) & 0xFF) << 8);
+                            }
+                            v10 = getOffset(offset);
                         }
-                        v10 = ((v11 << 24) >> 24) + dword_51F018[v11 >> 8];
                         break;
                     case 4:
                     case 5:
-                        if (v7 == 4) {
-                            byte = *a2++;
-                            v13 = word_51F418[byte];
-                        } else {
-                            v13 = *(unsigned short*)a2;
-                            a2 += 2;
-                        }
+                        if (1) {
+                            uint16_t offset;
+                            if (v7 == 4) {
+                                byte = *a2++;
+                                offset = word_51F418[byte];
+                            } else {
+                                offset = loadUInt16LE(a2);
+                                a2 += 2;
+                            }
 
-                        v10 = ((v13 << 24) >> 24) + dword_51F018[v13 >> 8] + (gMovieDirectDrawSurfaceBuffer2 - gMovieDirectDrawSurfaceBuffer1);
+                            v10 = getOffset(offset) + (gMovieDirectDrawSurfaceBuffer2 - gMovieDirectDrawSurfaceBuffer1);
+                        }
                         break;
                     }
 
                     value2 = _mveBW;
 
-                    for (i = 0; i < 8; i++) {
-                        src_ptr = (unsigned int*)(dest + v10);
-                        dest_ptr = (unsigned int*)dest;
-
-                        dest_ptr[0] = src_ptr[0];
-                        dest_ptr[1] = src_ptr[1];
-
+                    for (i = 0; i < 8; ++i) {
+                        memcpy(dest, dest + v10, 8);
                         dest += value2;
                     }
 
@@ -2669,11 +2670,8 @@ static void _nfPkDecomp(unsigned char* a1, unsigned char* a2, int a3, int a4, in
                 case 11:
                     value2 = _mveBW;
 
-                    src_ptr = (unsigned int*)a2;
-                    for (i = 0; i < 8; i++) {
-                        dest_ptr = (unsigned int*)dest;
-                        dest_ptr[0] = src_ptr[i * 2];
-                        dest_ptr[1] = src_ptr[i * 2 + 1];
+                    for (i = 0; i < 32; i += 4) {
+                        memcpy(dest, &a2[i * 2], 8);
                         dest += value2;
                     }
 
@@ -2763,7 +2761,7 @@ static void _nfPkDecomp(unsigned char* a1, unsigned char* a2, int a3, int a4, in
                         value1 = byte | (byte << 8) | (byte << 16) | (byte << 24);
                         value2 = value1;
                     } else {
-                        byte = *(unsigned short*)a2;
+                        byte = loadUInt16LE(a2);
                         a2 += 2;
                         value1 = byte | (byte << 16);
                         value2 = value1;
@@ -2794,4 +2792,19 @@ static void _nfPkDecomp(unsigned char* a1, unsigned char* a2, int a3, int a4, in
     }
 }
 
+constexpr uint16_t loadUInt16LE(const uint8_t* b)
+{
+    return (b[1] << 8) | b[0];
+}
+
+constexpr uint32_t loadUInt32LE(const uint8_t* b)
+{
+    return (b[3] << 24) | (b[2] << 16) | (b[1] << 8) | b[0];
+}
+
+uint8_t getOffset(uint16_t v)
+{
+    return static_cast<int8_t>(v & 0xFF) + dword_51F018[v >> 8];
+}
+
 } // namespace fallout