Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix some UB in movie_lib.cc #117

Merged
merged 5 commits into from
Mar 5, 2024
Merged

Fix some UB in movie_lib.cc #117

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
560d7e6
Fix some UB in movie_lib.cc
glebm Oct 29, 2023
11b2834
Merge branch 'main' into fix-ub
alexbatalov Mar 5, 2024
12546a7
Reorganize
alexbatalov Mar 5, 2024
120aece
Reorganize
alexbatalov Mar 5, 2024
08132a2
Remove constexpr
alexbatalov Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 47 additions & 34 deletions src/movie_lib.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "movie_lib.h"

#include <assert.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;

Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Copy link
Contributor Author

@glebm glebm Jun 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexbatalov As I mentioned in #117 (comment), the return type here (changed from auto to uint8_t when merging) is incorrect, it should be int or int32_t. This caused #136.

{
return static_cast<int8_t>(v & 0xFF) + dword_51F018[v >> 8];
}

} // namespace fallout
Loading