String safety (#1548)

* Array size UB fixes

* Fix ShieldD

* Remove (almost) all unsafe strcpy calls

Bunch of macros. C arrays are easy enough and just need a different call. For various cases where a char* is passed around bare, I've made a TEXT_SPAN macro that can store a length too for bounds checking.

* Move crash handling in safe string operations to separate TU

* strcat safe version

* sprintf made safe too

* Fix compile
This commit is contained in:
Pieter-Jan Briers
2026-05-24 18:43:00 +02:00
committed by GitHub
parent af162bbd0a
commit a6376368ee
100 changed files with 781 additions and 546 deletions
+1 -1
View File
@@ -77,7 +77,7 @@ private:
#define PLAYER_CREATE_ANM_HEAP_F(heap, type, fmt, ...) \
{ \
char pcah_name_buf[32]; \
sprintf(pcah_name_buf, fmt, ##__VA_ARGS__); \
snprintf(pcah_name_buf, sizeof(pcah_name_buf), fmt, ##__VA_ARGS__); \
(heap).createHeap(type, pcah_name_buf); \
\
}
+10 -12
View File
@@ -444,8 +444,7 @@ public:
/* 0x1C */ f32 mFovy;
/* 0x20 */ f32 mBank;
/* 0x24 */ fopAc_ac_c* mRelActor;
/* 0x28 */ char mRelUseMask;
/* 0x29 */ char field_0x29;
/* 0x28 */ char mRelUseMask[4];
/* 0x2C */ int mTimer;
/* 0x30 */ bool field_0x30;
/* 0x34 */ cXyz mBasePos;
@@ -526,7 +525,7 @@ public:
/* 0x3C */ fopAc_ac_c* field_0x3c;
/* 0x40 */ fopAc_ac_c* field_0x40;
/* 0x44 */ fpc_ProcID field_0x44;
/* 0x48 */ char field_0x48;
/* 0x48 */ char field_0x48[4];
/* 0x4C */ int field_0x4c;
};
@@ -541,11 +540,7 @@ public:
/* 0x3C */ f32 field_0x3c;
/* 0x40 */ fopAc_ac_c* mRelActor;
/* 0x44 */ fpc_ProcID mRelActorID;
/* 0x48 */ char mRelUseMask;
/* 0x49 */ char field_0x49;
/* 0x4A */ char field_0x4a;
/* 0x4B */ char field_0x4b;
/* 0x4C */ u8 field_0x4c[4];
/* 0x48 */ char mRelUseMask[8];
/* 0x50 */ int mTimer;
/* 0x54 */ int mTransType;
/* 0x58 */ f32 mCushion;
@@ -696,8 +691,7 @@ public:
/* 0x24 */ f32* field_0x24;
/* 0x28 */ f32 field_0x28;
/* 0x2C */ fopAc_ac_c* mRelActor;
/* 0x30 */ char mRelUseMask;
/* 0x31 */ char field_0x31;
/* 0x30 */ char mRelUseMask[4];
/* 0x34 */ int mTimer;
/* 0x38 */ int field_0x38;
/* 0x3C */ int mChoice;
@@ -780,8 +774,7 @@ public:
/* 0xAC */ f32 field_0xac;
/* 0xB0 */ fopAc_ac_c* mRelActor;
/* 0xB4 */ fpc_ProcID mRelActorID;
/* 0xB8 */ char mRelUseMask;
/* 0xB9 */ char field_0xb9;
/* 0xB8 */ char mRelUseMask[4];
/* 0xBC */ f32 mCushion;
/* 0xC0 */ u32 field_0xc0[6];
};
@@ -917,7 +910,12 @@ public:
char* getEvStringPntData(char*, char*);
char* getEvStringPntData(char*);
bool getEvXyzData(cXyz*, char*, cXyz);
#if TARGET_PC
template<size_t N>
bool getEvStringData(char (&)[N], char*, char*);
#else
bool getEvStringData(char*, char*, char*);
#endif
fopAc_ac_c* getEvActor(char*);
fopAc_ac_c* getEvActor(char*, char*);
bool pauseEvCamera();
+2 -2
View File
@@ -1851,7 +1851,7 @@ inline u16 dComIfGs_getDeathCount() {
}
#endif
inline char* dComIfGs_getPlayerName() {
inline TEXT_SPAN dComIfGs_getPlayerName() {
return g_dComIfG_gameInfo.info.getPlayer().getPlayerInfo().getPlayerName();
}
@@ -1859,7 +1859,7 @@ inline void dComIfGs_setPlayerName(const char* i_name) {
g_dComIfG_gameInfo.info.getPlayer().getPlayerInfo().setPlayerName(i_name);
}
inline char* dComIfGs_getHorseName() {
inline TEXT_SPAN dComIfGs_getHorseName() {
return g_dComIfG_gameInfo.info.getPlayer().getPlayerInfo().getHorseName();
}
+4 -4
View File
@@ -46,10 +46,10 @@ private:
/* 0x22 */ u8 field_0x22;
/* 0x24 */ CPaneMgrAlpha* mDatBase;
/* 0x28 */ CPaneMgrAlpha* mNoDatBase;
/* 0x2C */ char* mPlayerName;
/* 0x30 */ char* mSaveDate;
/* 0x34 */ char* mPlayTime;
/* 0x38 */ char* mSaveStatus;
/* 0x2C */ TEXT_SPAN mPlayerName;
/* 0x30 */ TEXT_SPAN mSaveDate;
/* 0x34 */ TEXT_SPAN mPlayTime;
/* 0x38 */ TEXT_SPAN mSaveStatus;
};
typedef void (dFile_info_c::*warningFunc)(void);
+3 -3
View File
@@ -565,7 +565,7 @@ public:
/* 0x0130 */ int field_0x0130;
/* 0x0134 */ int field_0x0134;
/* 0x0138 */ CPaneMgrAlpha* mErrorMsgTxtPane[2];
/* 0x0140 */ char* mErrorMsgStringPtr[2];
/* 0x0140 */ TEXT_SPAN mErrorMsgStringPtr[2];
/* 0x0148 */ u8 mErrorTxtDispIdx;
/* 0x0149 */ u8 field_0x0149;
/* 0x014A */ bool field_0x014a;
@@ -609,7 +609,7 @@ public:
/* 0x020A */ u8 mFadeTimer;
/* 0x020B */ u8 field_0x020b;
/* 0x020C */ CPaneMgrAlpha* mHeaderTxtPane[2];
/* 0x0214 */ char* mHeaderStringPtr[2];
/* 0x0214 */ TEXT_SPAN mHeaderStringPtr[2];
/* 0x021C */ u8 mHeaderTxtDispIdx;
/* 0x021D */ u8 field_0x021d;
/* 0x021E */ u8 field_0x021e;
@@ -626,7 +626,7 @@ public:
/* 0x024B */ u8 field_0x024b;
/* 0x024C */ u8 field_0x024c;
/* 0x024B */ u8 field_0x024d[3];
/* 0x0250 */ char* mModoruStringPtr;
/* 0x0250 */ TEXT_SPAN mModoruStringPtr;
/* 0x0254 */ STControl* stick;
/* 0x0258 */ u8 mIsDataNew[3];
/* 0x025B */ u8 mIsNoData[3];
+2 -2
View File
@@ -313,7 +313,7 @@ private:
/* 0x00B8 */ int field_0xb8;
/* 0x00BC */ int field_0xbc;
/* 0x00C0 */ CPaneMgrAlpha* mpErrTxtPane[2];
/* 0x00C8 */ char* mpErrTxt[2];
/* 0x00C8 */ TEXT_SPAN mpErrTxt[2];
/* 0x00D0 */ u8 mErrTxtType;
/* 0x00D1 */ u8 mErrTxtAnmComplete;
/* 0x00D2 */ u8 field_0xd2;
@@ -344,7 +344,7 @@ private:
/* 0x0160 */ J2DAnmTevRegKey* field_0x160;
/* 0x0164 */ int field_0x164;
/* 0x0168 */ CPaneMgrAlpha* mpHeaderTxtPane[2];
/* 0x0170 */ char* mpHeaderTxt[2];
/* 0x0170 */ TEXT_SPAN mpHeaderTxt[2];
/* 0x0178 */ u8 mHeaderTxtType; // 0: Select Menu 1: YesNo Menu
/* 0x0179 */ u8 mHeaderAnmComplete;
/* 0x017A */ u8 field_0x17a;
+6 -6
View File
@@ -67,9 +67,9 @@ public:
s16 decFloatingMessageTimer();
void resetFloatingMessage();
void decMsgKeyWaitTimer();
void getString(u32 i_stringID, char* o_string, JMSMesgEntry_c* i_msgEntry);
void getStringKana(u32 i_stringID, char* o_string, JMSMesgEntry_c* i_msgEntry);
void getStringKanji(u32 i_stringID, char* o_string, JMSMesgEntry_c* i_msgEntry);
void getString(u32 i_stringID, TEXT_SPAN o_string, JMSMesgEntry_c* i_msgEntry);
void getStringKana(u32 i_stringID, TEXT_SPAN o_string, JMSMesgEntry_c* i_msgEntry);
void getStringKanji(u32 i_stringID, TEXT_SPAN o_string, JMSMesgEntry_c* i_msgEntry);
f32 getStringLength(J2DTextBox* i_textbox, char* i_string);
f32 getStringLength(JUTFont* i_font, f32 param_2, f32 param_3, char* i_string);
void onDirectUseItem(int);
@@ -348,15 +348,15 @@ inline CPaneMgr* dMeter2Info_getMeterItemPanePtr(s32 i_idx) {
return g_meter2_info.getMeterItemPanePtr(i_idx);
}
inline void dMeter2Info_getString(u32 i_stringID, char* o_string, JMSMesgEntry_c* i_msgEntry) {
inline void dMeter2Info_getString(u32 i_stringID, TEXT_SPAN o_string, JMSMesgEntry_c* i_msgEntry) {
g_meter2_info.getString(i_stringID, o_string, i_msgEntry);
}
inline void dMeter2Info_getStringKanji(u32 i_stringID, char* o_string, JMSMesgEntry_c* i_msgEntry) {
inline void dMeter2Info_getStringKanji(u32 i_stringID, TEXT_SPAN o_string, JMSMesgEntry_c* i_msgEntry) {
g_meter2_info.getStringKanji(i_stringID, o_string, i_msgEntry);
}
inline void dMeter2Info_getStringKana(u32 i_stringID, char* o_string, JMSMesgEntry_c* i_msgEntry) {
inline void dMeter2Info_getStringKana(u32 i_stringID, TEXT_SPAN o_string, JMSMesgEntry_c* i_msgEntry) {
g_meter2_info.getStringKana(i_stringID, o_string, i_msgEntry);
}
+8 -7
View File
@@ -5,6 +5,7 @@
#include "JSystem/JMessage/JMessage.h"
#include "SSystem/SComponent/c_xyz.h"
#include "dusk/endian.h"
#include "dusk/string.hpp"
#if REGION_JPN
#define D_MSG_CLASS_PAGE_CNT_MAX 30
@@ -67,7 +68,7 @@ struct jmessage_tReference : public JMessage::TReference {
void pageSend();
void selectMessage();
void inputNumber();
char* getWord(int);
TEXT_SPAN getWord(int);
void resetWord();
void setCharactor(u16);
void addCharactor(u16);
@@ -228,11 +229,11 @@ struct jmessage_tReference : public JMessage::TReference {
f32 getSelRubyCharSpace() { return mSelRubyCharSpace; }
f32 getRubySize() { return mRubySize; }
f32 getRubyCharSpace() { return mRubyCharSpace; }
char* getSelTextPtr(int idx) { return mSelText[idx]; }
char* getSelRubyPtr(int idx) { return mSelRuby[idx]; }
char* getTextPtr() { return mText; }
char* getTextSPtr() { return mTextS; }
char* getRubyPtr() { return mRuby; }
TEXT_SPAN getSelTextPtr(int idx) { return mSelText[idx]; }
TEXT_SPAN getSelRubyPtr(int idx) { return mSelRuby[idx]; }
TEXT_SPAN getTextPtr() { return mText; }
TEXT_SPAN getTextSPtr() { return mTextS; }
TEXT_SPAN getRubyPtr() { return mRuby; }
u8 getSelectRubyFlag() { return mSelectRubyFlag; }
f32 getSelTBoxWidth() { return mSelTBoxWidth; }
u8 getSelectPos() { return mSelectPos; }
@@ -463,7 +464,7 @@ struct jmessage_tRenderingProcessor : public JMessage::TRenderingProcessor {
f32 getLineLength(int);
void do_strcat(char*, bool, bool, bool);
void do_rubyset(void const*, u32);
void do_rubystrcat(char*, char*, f32, f32);
void do_rubystrcat(char*, TEXT_SPAN, f32, f32);
void do_name1();
void do_numset(s16);
void push_word();
+7 -7
View File
@@ -53,8 +53,8 @@ public:
void demoMessageGroupLocal();
void endFlowGroupLocal();
void changeGroupLocal(s16);
bool getStringLocal(u32, J2DTextBox*, J2DTextBox*, JUTFont*, COutFont_c*, char*,
char*, char*, s16*);
bool getStringLocal(u32, J2DTextBox*, J2DTextBox*, JUTFont*, COutFont_c*, TEXT_SPAN,
TEXT_SPAN, TEXT_SPAN, s16*);
bool isGetItemMessage();
bool isKanbanMessage();
bool isHowlMessage();
@@ -121,7 +121,7 @@ public:
static void endFlowGroup();
static void changeGroup(s16);
static bool getString(u32, J2DTextBox*, J2DTextBox*, JUTFont*, COutFont_c*,
char*, char*, char*, s16*);
TEXT_SPAN, TEXT_SPAN, TEXT_SPAN, s16*);
static void* getMsgDtPtr();
static void setProcessID(fpc_ProcID);
static msg_class* getActor();
@@ -246,12 +246,12 @@ public:
static void setWord(const char* i_word);
void setWordLocal(const char* i_word) {
strcpy(mWord, i_word);
SAFE_STRCPY(mWord, i_word);
}
static void setSelectWord(int i_no, const char* i_word);
void setSelectWordLocal(int i_no, const char* i_word) {
strcpy(mSelectWord[i_no], i_word);
SAFE_STRCPY(mSelectWord[i_no], i_word);
}
jmessage_tSequenceProcessor* getSequenceProcessor() { return mpSeqProc; }
@@ -426,8 +426,8 @@ inline void dMsgObject_setTalkActor(fopAc_ac_c* actor) {
}
inline bool dMsgObject_getString(u32 i_msgId, J2DTextBox* i_tbox, J2DTextBox* i_rubyTbox,
JUTFont* i_font, COutFont_c* i_outFont, char* o_text,
char* o_ruby, char* o_textS, s16* param_8) {
JUTFont* i_font, COutFont_c* i_outFont, TEXT_SPAN o_text,
TEXT_SPAN o_ruby, TEXT_SPAN o_textS, s16* param_8) {
return dMsgObject_getMsgObjectClass()->getString(i_msgId, i_tbox, i_rubyTbox, i_font, i_outFont, o_text, o_ruby,
o_textS, param_8);
}
+2 -2
View File
@@ -20,12 +20,12 @@ public:
f32 getStringPageLocal(u32, u8, u8, J2DTextBox*, J2DTextBox*, JUTFont*,
COutFont_c*, u8);
u8 getPageMax(int);
f32 getMessageLocal(u32, char*);
f32 getMessageLocal(u32, TEXT_SPAN);
virtual f32 getString(u32, J2DTextBox*, J2DTextBox*, JUTFont*, COutFont_c*, u8);
virtual f32 getStringPage(u32, u8, u8, J2DTextBox*, J2DTextBox*, JUTFont*, COutFont_c*,
u8);
virtual f32 getMessage(u32, char*);
virtual f32 getMessage(u32, TEXT_SPAN);
virtual void resetStringLocal(J2DTextBox*);
virtual void drawOutFontLocal(J2DTextBox*, f32);
virtual void drawFontLocal(J2DTextBox*, u8, f32, f32, f32, f32, u32, u8);
+2 -2
View File
@@ -6,14 +6,14 @@
class dMsgUnit_c {
public:
dMsgUnit_c();
void setTag(int, int, char*, bool);
void setTag(int, int, TEXT_SPAN, bool);
virtual ~dMsgUnit_c();
};
extern dMsgUnit_c g_msg_unit;
inline void dMsgUnit_setTag(int param_0, int param_1, char* param_2) {
inline void dMsgUnit_setTag(int param_0, int param_1, TEXT_SPAN param_2) {
g_msg_unit.setTag(param_0, param_1, param_2, true);
}
+3 -3
View File
@@ -127,7 +127,7 @@ public:
u8 isInputEnd() { return mIsInputEnd; }
char* getInputStrPtr() { return mInputStr; }
void hideIcon() { mSelIcon->setAlphaRate(0.0f); }
void setNextNameStr(char* i_name) { strcpy(mNextNameStr,i_name); }
void setNextNameStr(char* i_name) { SAFE_STRCPY(mNextNameStr,i_name); }
void draw() { _draw(); }
private:
@@ -140,9 +140,9 @@ private:
/* 0x02C */ J2DAnmTextureSRTKey* mCursorTexKey;
/* 0x030 */ int mCurTexAnmF;
/* 0x034 */ CPaneMgrAlpha* mNameCursor[8];
/* 0x054 */ char* mNameText[8];
/* 0x054 */ TEXT_SPAN mNameText[8];
/* 0x074 */ CPaneMgr* mMojiIcon[65];
/* 0x178 */ char* mMojiText[65];
/* 0x178 */ TEXT_SPAN mMojiText[65];
/* 0x27C */ J2DPane* mMojiPane;
/* 0x280 */ J2DPane* mMenuPane;
/* 0x284 */ CPaneMgr* mMenuIcon[4];
+9 -5
View File
@@ -488,21 +488,25 @@ public:
}
#if TARGET_PC
u16 getDeathCount() const { return mDeathCount; }
#endif
TEXT_SPAN getPlayerName() const { return const_cast<char(&)[17]>(mPlayerName); }
#else
char* getPlayerName() const { return const_cast<char*>(mPlayerName); }
#endif
void setPlayerName(const char* i_name) {
#if AVOID_UB
strncpy(mPlayerName, i_name, sizeof(mPlayerName) - 1);
mPlayerName[sizeof(mPlayerName) - 1] = '\0';
dusk::SafeStringCopyTruncate(mPlayerName, i_name);
#else
strcpy(mPlayerName, i_name);
#endif
}
#if TARGET_PC
TEXT_SPAN getHorseName() const { return const_cast<char(&)[17]>(mHorseName); }
#else
char* getHorseName() const { return const_cast<char*>(mHorseName); }
#endif
void setHorseName(const char* i_name) {
#if AVOID_UB
strncpy(mHorseName, i_name, sizeof(mHorseName) - 1);
mHorseName[sizeof(mHorseName) - 1] = '\0';
dusk::SafeStringCopyTruncate(mHorseName, i_name);
#else
strcpy(mHorseName, i_name);
#endif
+1 -1
View File
@@ -1225,7 +1225,7 @@ public:
#endif
}
static JKRExpHeap* getMemoryBlockHeap(int i_no) { return mMemoryBlock[i_no]; }
static char* getDemoArcName() { return mDemoArcName; }
static TEXT_SPAN getDemoArcName() { return mDemoArcName; }
static void offNoChangeRoom() { mNoChangeRoom = false; }
static void onNoChangeRoom() { mNoChangeRoom = true; }
+98 -28
View File
@@ -1,22 +1,55 @@
#ifndef DUSK_STRING_HPP
#define DUSK_STRING_HPP
#include "global.h"
#include <cstring>
#include <dolphin/os.h>
#include <cstdarg>
namespace dusk {
inline void strncpyProxy(char* dst, const char* src, size_t count) {
#if _MSC_VER
#pragma warning(push)
#pragma warning(disable : 4996)
struct TextSpan {
char* buffer;
size_t size;
constexpr operator char*() const {
return buffer;
}
constexpr TextSpan(char* buffer, size_t size) : buffer(buffer), size(size) { }
template<size_t BufSize>
constexpr TextSpan(char (&buffer)[BufSize]) : buffer(buffer), size(BufSize) {
}
constexpr TextSpan() : buffer(nullptr), size(0) { }
constexpr TextSpan operator++(int) {
const auto prev = *this;
if (size > 0) [[likely]] {
size--;
}
buffer++;
return prev;
}
constexpr char& operator*() const {
if (size == 0) [[unlikely]] {
CrashSpawnEmpty();
}
return *buffer;
}
private:
static void CrashSpawnEmpty();
};
#if TARGET_PC
#define TEXT_SPAN dusk::TextSpan
#else
#define TEXT_SPAN char*
#endif
strncpy(dst, src, count);
#if _MSC_VER
#pragma warning(pop)
#endif
}
void SafeStringCopyTruncate(char* buffer, size_t bufSize, const char* src);
/**
* Copy a string to a fixed-size array.
@@ -25,13 +58,28 @@ inline void strncpyProxy(char* dst, const char* src, size_t count) {
template <size_t BufSize>
void SafeStringCopyTruncate(char (&buffer)[BufSize], const char* src) {
static_assert(BufSize > 0, "Target buffer cannot be size zero");
SafeStringCopyTruncate(buffer, BufSize, src);
}
if (buffer == src) {
CRASH("Cannot copy string to same buffer");
}
void SafeStringCopy(char* buffer, size_t bufSize, const char* src);
void SafeStringCat(char* buffer, size_t bufSize, const char* src);
int SafeStringVPrintf(char* buffer, size_t bufSize, const char* src, std::va_list args);
strncpyProxy(buffer, src, BufSize);
buffer[BufSize - 1] = 0;
inline void SafeStringCopy(TextSpan dst, const char* src) {
SafeStringCopy(dst.buffer, dst.size, src);
}
inline void SafeStringCat(TextSpan dst, const char* src) {
SafeStringCat(dst.buffer, dst.size, src);
}
inline int SafeStringPrintf(TextSpan dst, const char* format, ...) {
std::va_list args;
va_start(args, format);
const auto ret = SafeStringVPrintf(dst.buffer, dst.size, format, args);
va_end(args);
return ret;
}
/**
@@ -41,18 +89,40 @@ void SafeStringCopyTruncate(char (&buffer)[BufSize], const char* src) {
template <size_t BufSize>
void SafeStringCopy(char (&buffer)[BufSize], const char* src) {
static_assert(BufSize > 0, "Target buffer cannot be size zero");
if (buffer == src) {
CRASH("Cannot copy string to same buffer");
}
if (strlen(src) > BufSize - 1) {
CRASH("Destination buffer too small!");
}
strncpyProxy(buffer, src, BufSize);
buffer[BufSize - 1] = 0;
SafeStringCopy(buffer, BufSize, src);
}
template <size_t BufSize>
void SafeStringCat(char (&buffer)[BufSize], const char* src) {
static_assert(BufSize > 0, "Target buffer cannot be size zero");
SafeStringCat(buffer, BufSize, src);
}
template <size_t BufSize>
int SafeStringPrintf(char (&buffer)[BufSize], const char* format, ...) {
static_assert(BufSize > 0, "Target buffer cannot be size zero");
std::va_list args;
va_start(args, format);
const auto ret = SafeStringVPrintf(buffer, BufSize, format, args);
va_end(args);
return ret;
}
#if TARGET_PC
#define SAFE_STRCPY dusk::SafeStringCopy
#define SAFE_STRCAT dusk::SafeStringCat
#define SAFE_SPRINTF dusk::SafeStringPrintf
#define SAFE_STRCPY_BOUNDED dusk::SafeStringCopy
#define SAFE_STRCAT_BOUNDED dusk::SafeStringCat
#else
#define SAFE_STRCPY strcpy
#define SAFE_STRCAT strcat
#define SAFE_SPRINTF sprintf
#define SAFE_STRCPY_BOUNDED strcpy
#define SAFE_STRCPY_BOUNDED strcat
#endif
}
#endif // DUSK_STRING_HPP
+1 -1
View File
@@ -50,7 +50,7 @@ fpc_ProcID fopMsgM_messageSet(u32 i_msgIdx, fopAc_ac_c* i_talkActor, u32 param_2
fpc_ProcID fopMsgM_messageSet(u32 i_msgIdx, u32 param_1);
fpc_ProcID fopMsgM_messageSetDemo(u32 i_msgidx);
msg_class* fopMsgM_SearchByID(fpc_ProcID i_id);
char* fopMsgM_messageGet(char* i_stringBuf, u32 i_msgId);
TEXT_SPAN fopMsgM_messageGet(TEXT_SPAN i_stringBuf, u32 i_msgId);
fpc_ProcID fop_Timer_create(s16 i_procName, u8 i_mode, u32 i_limitMs, u8 i_type, u8 param_4,
f32 param_5, f32 param_6, f32 param_7, f32 param_8,
fopMsgCreateFunc i_createFunc);