port UBFIXes from emerald

7 files changed, 62 insertions, 7 deletions

diff --git a/src/battle_ai_script_commands.c b/src/battle_ai_script_commands.c
index 952c1487f..a500d591b 100644
--- a/src/battle_ai_script_commands.c
+++ b/src/battle_ai_script_commands.c
@@ -1362,7 +1362,12 @@ static void Cmd_if_status_not_in_party(void)
 
         // everytime the status is found, the AI's logic jumps further and further past its intended destination. this results in a broken AI macro and is probably why it is unused.
         if (species != SPECIES_NONE && species != SPECIES_EGG && hp != 0 && status == statusToCompareTo)
+        {
             sAIScriptPtr += 10; // doesnt return?
+            #ifdef UBFIX
+            return;
+            #endif
+        }
     }
     sAIScriptPtr = T1_READ_PTR(sAIScriptPtr + 6);
 }
diff --git a/src/easy_chat.c b/src/easy_chat.c
index 4360dc72e..55bc104db 100644
--- a/src/easy_chat.c
+++ b/src/easy_chat.c
@@ -459,12 +459,17 @@ void InitEasyChatPhrases(void)
             gSaveBlock1Ptr->mail[i].words[j] = EC_WORD_UNDEFINED;
     }
 
+#ifndef UBFIX
     // BUG: This is supposed to clear 64 bits, but this loop is clearing 64 bytes.
     // However, this bug has no resulting effect on gameplay because only the
     // Mauville old man data is corrupted, which is initialized directly after
     // this function is called when starting a new game.
     for (i = 0; i < 64; i++)
         gSaveBlock1Ptr->additionalPhrases[i] = 0;
+#else
+    for (i = 0; i < NELEMS(gSaveBlock1Ptr->additionalPhrases); i++)
+        gSaveBlock1Ptr->additionalPhrases[i] = 0;
+#endif
 }
 
 void EC_ResetMEventProfileMaybe(void)
diff --git a/src/event_object_movement.c b/src/event_object_movement.c
index a671ada34..14914bf92 100644
--- a/src/event_object_movement.c
+++ b/src/event_object_movement.c
@@ -2561,7 +2561,13 @@ const u8 *GetObjectEventScriptPointerByObjectEventId(u8 objectEventId)
 
 static u16 GetObjectEventFlagIdByLocalIdAndMap(u8 localId, u8 mapNum, u8 mapGroup)
 {
-    return GetObjectEventTemplateByLocalIdAndMap(localId, mapNum, mapGroup)->flagId;
+    struct ObjectEventTemplate *obj = GetObjectEventTemplateByLocalIdAndMap(localId, mapNum, mapGroup);
+#ifdef UBFIX
+    // BUG: The function may return NULL, and attempting to read from NULL may freeze the game using modern compilers.
+    if (obj == NULL)
+        return 0;
+#endif // UBFIX
+    return obj->flagId;
 }
 
 static u16 GetObjectEventFlagIdByObjectEventId(u8 objectEventId)
diff --git a/src/field_effect.c b/src/field_effect.c
index 4cab3cff8..4ccf50b0b 100644
--- a/src/field_effect.c
+++ b/src/field_effect.c
@@ -3666,8 +3666,16 @@ static void Task_MoveDeoxysRock_Step(u8 taskId)
     case 0:
         data[4] = sprite->pos1.x << 4;
         data[5] = sprite->pos1.y << 4;
-        data[6] = ((data[2] << 4) - data[4]) / data[8];
-        data[7] = ((data[3] << 4) - data[5]) / data[8];
+
+        // UB: Possible divide by zero
+        #ifdef UBFIX
+        #define DIVISOR (data[8] ? data[8] : 1);
+        #else
+        #define DIVISOR (data[8])
+        #endif
+
+        data[6] = ((data[2] << 4) - data[4]) / DIVISOR;
+        data[7] = ((data[3] << 4) - data[5]) / DIVISOR;
         data[0]++;
         // fallthrough
     case 1:
diff --git a/src/fieldmap.c b/src/fieldmap.c
index 7ece8b622..400fc4cff 100644
--- a/src/fieldmap.c
+++ b/src/fieldmap.c
@@ -528,10 +528,14 @@ static bool32 SavedMapViewIsEmpty(void)
     u16 i;
     u32 marker = 0;
 
+#ifndef UBFIX
     // BUG: This loop extends past the bounds of the mapView array. Its size is only 0x100.
     for (i = 0; i < 0x200; i++)
         marker |= gSaveBlock2Ptr->mapView[i];
-
+#else
+    for (i = 0; i < NELEMS(gSaveBlock2Ptr->mapView); i++)
+        marker |= gSaveBlock2Ptr->mapView[i];
+#endif
     if (marker == 0)
         return TRUE;
     else
@@ -746,14 +750,32 @@ struct MapConnection *sub_8059600(u8 direction, s32 x, s32 y)
 {
     s32 count;
     struct MapConnection *connection;
+    const struct MapConnections *connections = gMapHeader.connections;
     s32 i;
-    count = gMapHeader.connections->count;
-    connection = gMapHeader.connections->connections;
+    // UB: Multiple possible null dereferences
+#ifdef UBFIX
+    if (connections != NULL)
+    {
+        count = connections->count;
+        connection = connections->connections;
+        if (connection != NULL)
+        {
+            for (i = 0; i < count; i++, connection++)
+            {
+                if (connection->direction == direction && sub_8059658(direction, x, y, connection) == TRUE)
+                    return connection;
+            }
+        }
+    }
+#else
+    count = connections->count;
+    connection = connections->connections;
     for (i = 0; i < count; i++, connection++)
     {
         if (connection->direction == direction && sub_8059658(direction, x, y, connection) == TRUE)
             return connection;
     }
+#endif
     return NULL;
 
 }
diff --git a/src/pokemon_storage_system_7.c b/src/pokemon_storage_system_7.c
index 3d0472230..a9b538cae 100644
--- a/src/pokemon_storage_system_7.c
+++ b/src/pokemon_storage_system_7.c
@@ -473,8 +473,13 @@ static void sub_80957C8(void)
         for (j = sMoveMonsPtr->minRow; j < rowCount; j++)
         {
             struct BoxPokemon *boxMon = GetBoxedMonPtr(boxId, boxPosition);
-
+            // UB: possible null dereference
+#ifdef UBFIX
+            if (boxMon != NULL)
+                sMoveMonsPtr->boxMons[monArrayId] = *boxMon;
+#else
             sMoveMonsPtr->boxMons[monArrayId] = *boxMon;
+#endif
             monArrayId++;
             boxPosition++;
         }
diff --git a/src/union_room.c b/src/union_room.c
index 575fcd60e..1fb7ee849 100644
--- a/src/union_room.c
+++ b/src/union_room.c
@@ -1628,7 +1628,11 @@ static bool32 IsPartnerActivityAcceptable(u32 activity, u32 group)
     if (group == 0xFF)
         return TRUE;
 
+    #ifndef UBFIX
     if (group <= NELEMS(sAcceptedActivityIds)) // UB: <= may access data outside the array
+    #else
+    if (group < NELEMS(sAcceptedActivityIds))
+    #endif
     {
         const u8 *bytes = sAcceptedActivityIds[group];