change cheat system name buffer management to use std::string to avoid issues caused by returning the name buffer to the user who could then use it in ways the cheat system didn't expect ultimately leading to invalid strcpy(X,X) calls and possible use of buffers after freeing. this will break all frontends probably (except for windows which I fixed at this time). fixes #512

This commit is contained in:
zeromus 2022-08-19 12:37:05 -04:00
parent 3eddaf8052
commit c37f86dbd1
7 changed files with 51 additions and 69 deletions

View File

@ -152,28 +152,23 @@ int FCEU_CalcCheatAffectedBytes(uint32 address, uint32 size) {
return count;
}
static int AddCheatEntry(const char *name, uint32 addr, uint8 val, int compare, int status, int type);
static void AddCheatEntry(const char *name, uint32 addr, uint8 val, int compare, int status, int type);
static void CheatMemErr(void)
{
FCEUD_PrintError("Error allocating memory for cheat data.");
}
static int AddCheatEntry(const char *name, uint32 addr, uint8 val, int compare, int status, int type)
static void AddCheatEntry(const char *name, uint32 addr, uint8 val, int compare, int status, int type)
{
struct CHEATF *temp;
if(!(temp = (struct CHEATF *)FCEU_dmalloc(sizeof(struct CHEATF))))
{
CheatMemErr();
return(0);
}
CHEATF *temp = new CHEATF();
temp->name = strcpy((char*) FCEU_dmalloc(strlen(name) + 1), name);
temp->name = name;
temp->addr = addr;
temp->val = val;
temp->status = status;
temp->compare = compare;
temp->type = type;
temp->next = 0;
temp->next = nullptr;
if(cheats)
{
@ -182,8 +177,6 @@ static int AddCheatEntry(const char *name, uint32 addr, uint8 val, int compare,
}
else
cheats = cheatsl = temp;
return (1);
}
/* The "override_existing" parameter is used only in cheat dialog import.
@ -306,14 +299,13 @@ void FCEU_SaveGameCheats(FILE* fp, int release)
fputc(':', fp);
if (next->compare >= 0)
fprintf(fp, "%04x:%02x:%02x:%s\n", next->addr, next->val, next->compare, next->name);
fprintf(fp, "%04x:%02x:%02x:%s\n", next->addr, next->val, next->compare, next->name.c_str());
else
fprintf(fp, "%04x:%02x:%s\n", next->addr, next->val, next->name);
fprintf(fp, "%04x:%02x:%s\n", next->addr, next->val, next->name.c_str());
if (release) free(next->name);
struct CHEATF *t = next;
next = next->next;
if (release) free(t);
if (release) delete t;
}
}
@ -333,8 +325,7 @@ void FCEU_FlushGameCheats(FILE *override, int nosave)
{
struct CHEATF *last=next;
next=next->next;
free(last->name);
free(last);
delete last;
if(!next) break;
}
cheats=cheatsl=0;
@ -379,9 +370,7 @@ void FCEU_FlushGameCheats(FILE *override, int nosave)
int FCEUI_AddCheat(const char *name, uint32 addr, uint8 val, int compare, int type)
{
if(!AddCheatEntry(name, addr, val, compare, 1, type))
return 0;
AddCheatEntry(name, addr, val, compare, 1, type);
savecheats = 1;
RebuildSubCheats();
@ -415,8 +404,7 @@ int FCEUI_DelCheat(uint32 which)
else
cheats=cheatsl=0; // No (more) cheats.
}
free(cur->name); // Now that all references to this cheat are removed,
free(cur); // free the memory.
delete cur; // free the memory.
break;
} // *END REMOVE THIS CHEAT*
@ -451,18 +439,18 @@ void FCEU_ApplyPeriodicCheats(void)
}
void FCEUI_ListCheats(int (*callb)(char *name, uint32 a, uint8 v, int compare, int s, int type, void *data), void *data)
void FCEUI_ListCheats(int (*callb)(const char *name, uint32 a, uint8 v, int compare, int s, int type, void *data), void *data)
{
struct CHEATF *next=cheats;
while(next)
{
if(!callb(next->name,next->addr,next->val,next->compare,next->status,next->type,data)) break;
if(!callb(next->name.c_str(),next->addr,next->val,next->compare,next->status,next->type,data)) break;
next=next->next;
}
}
int FCEUI_GetCheat(uint32 which, char **name, uint32 *a, uint8 *v, int *compare, int *s, int *type)
int FCEUI_GetCheat(uint32 which, std::string *name, uint32 *a, uint8 *v, int *compare, int *s, int *type)
{
struct CHEATF *next=cheats;
uint32 x=0;
@ -600,7 +588,7 @@ int FCEUI_DecodePAR(const char *str, int *a, int *v, int *c, int *type)
/* name can be NULL if the name isn't going to be changed. */
/* same goes for a, v, and s(except the values of each one must be <0) */
int FCEUI_SetCheat(uint32 which, const char *name, int32 a, int32 v, int c, int s, int type)
int FCEUI_SetCheat(uint32 which, const std::string *name, int32 a, int32 v, int c, int s, int type)
{
struct CHEATF *next = cheats;
uint32 x = 0;
@ -610,13 +598,7 @@ int FCEUI_SetCheat(uint32 which, const char *name, int32 a, int32 v, int c, int
if(x == which)
{
if(name)
{
char *t;
if((t = (char *)realloc(next->name, strlen(name) + 1)))
strcpy(next->name = t, name);
else
return 0;
}
next->name = *name;
if(a >= 0)
next->addr = a;
if(v >= 0)
@ -911,11 +893,7 @@ int FCEU_DeleteAllCheats(void)
while (cur)
{
next = cur->next;
if ( cur->name )
{
free(cur->name);
}
free(cur);
delete cur;
cur = next;
}
cheats = cheatsl = 0;

View File

@ -42,7 +42,7 @@ typedef struct {
struct CHEATF {
struct CHEATF *next;
char *name;
std::string name;
uint16 addr;
uint8 val;
int compare; /* -1 for no compare. */

View File

@ -207,10 +207,10 @@ void FCEUI_CheatSearchGetRange(uint32 first, uint32 last, int (*callb)(uint32 a,
void FCEUI_CheatSearchGet(int (*callb)(uint32 a, uint8 last, uint8 current, void *data), void *data);
void FCEUI_CheatSearchBegin(void);
void FCEUI_CheatSearchEnd(int type, uint8 v1, uint8 v2);
void FCEUI_ListCheats(int (*callb)(char *name, uint32 a, uint8 v, int compare, int s, int type, void *data), void *data);
void FCEUI_ListCheats(int (*callb)(const char *name, uint32 a, uint8 v, int compare, int s, int type, void *data), void *data);
int FCEUI_GetCheat(uint32 which, char **name, uint32 *a, uint8 *v, int *compare, int *s, int *type);
int FCEUI_SetCheat(uint32 which, const char *name, int32 a, int32 v, int compare,int s, int type);
int FCEUI_GetCheat(uint32 which, std::string *name, uint32 *a, uint8 *v, int *compare, int *s, int *type);
int FCEUI_SetCheat(uint32 which, const std::string *name, int32 a, int32 v, int compare,int s, int type);
void FCEUI_CheatSearchShowExcluded(void);
void FCEUI_CheatSearchSetCurrentAsOriginal(void);

View File

@ -199,7 +199,8 @@ static void ToggleCheat(int num)
static void ModifyCheat(int num)
{
char *name;
std::string name;
std::string *pName;
char buf[256];
uint32 A;
uint8 V;
@ -211,7 +212,7 @@ static void ModifyCheat(int num)
FCEUI_GetCheat(num, &name, &A, &V, &compare, &s, &type);
printf("Name [%s]: ",name);
printf("Name [%s]: ",name.c_str());
GetString(buf,256);
/* This obviously doesn't allow for cheats with no names. Bah. Who wants
@ -219,9 +220,9 @@ static void ModifyCheat(int num)
*/
if(buf[0])
name=buf; // Change name when FCEUI_SetCheat() is called.
pName=&name; // Change name when FCEUI_SetCheat() is called.
else
name=0; // Don't change name when FCEUI_SetCheat() is called.
pName=nullptr; // Don't change name when FCEUI_SetCheat() is called.
printf("Address [$%04x]: ",(unsigned int)A);
A=GetH16(A);
@ -240,7 +241,7 @@ static void ModifyCheat(int num)
if(t=='Y' || t=='y') s=1;
else if(t=='N' || t=='n') s=0;
FCEUI_SetCheat(num,name,A,V,compare,s,type);
FCEUI_SetCheat(num,pName,A,V,compare,s,type);
}
@ -320,7 +321,7 @@ static void AddCheat(void)
}
static int lid;
static int clistcallb(char *name, uint32 a, uint8 v, int compare, int s, int type, void *data)
static int clistcallb(const char *name, uint32 a, uint8 v, int compare, int s, int type, void *data)
{
char tmp[512];
int ret;

View File

@ -108,7 +108,7 @@ char *U8ToStr(uint8 a)
}
//int RedoCheatsCallB(char *name, uint32 a, uint8 v, int s) { //bbit edited: this commented out line was changed to the below for the new fceud
int RedoCheatsCallB(char *name, uint32 a, uint8 v, int c, int s, int type, void* data)
int RedoCheatsCallB(const char *name, uint32 a, uint8 v, int c, int s, int type, void* data)
{
char str[256] = { 0 };
GetCheatStr(str, a, v, c);
@ -123,7 +123,7 @@ int RedoCheatsCallB(char *name, uint32 a, uint8 v, int c, int s, int type, void*
else
lvi.iItem = SendDlgItemMessage(hCheat, IDC_LIST_CHEATS, LVM_INSERTITEM, 0, (LPARAM)&lvi);
lvi.iSubItem = 1;
lvi.pszText = name;
lvi.pszText = (LPSTR)name;
SendDlgItemMessage(hCheat, IDC_LIST_CHEATS, LVM_SETITEM, 0, (LPARAM)&lvi);
lvi.mask = LVIF_STATE;
@ -483,11 +483,12 @@ INT_PTR CALLBACK CheatConsoleCallB(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARA
lvi.stateMask = LVIS_STATEIMAGEMASK;
int tmpsel = SendDlgItemMessage(hCheat, IDC_LIST_CHEATS, LVM_GETNEXTITEM, -1, LVNI_ALL | LVNI_SELECTED);
char* name = ""; int s;
std::string name;
int s;
while (tmpsel != -1)
{
FCEUI_GetCheat(tmpsel, &name, NULL, NULL, NULL, &s, NULL);
FCEUI_SetCheat(tmpsel, name, -1, -1, -2, s ^= 1, 1);
FCEUI_SetCheat(tmpsel, &name, -1, -1, -2, s ^= 1, 1);
lvi.iItem = tmpsel;
lvi.state = INDEXTOSTATEIMAGEMASK(s ? 2 : 1);
@ -645,12 +646,14 @@ INT_PTR CALLBACK CheatConsoleCallB(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARA
if (selcheat < 0)
break;
char name[256]; uint32 a; uint8 v; int s; int c;
GetUICheatInfo(hwndDlg, name, &a, &v, &c);
std::string name; uint32 a; uint8 v; int s; int c;
char namebuf[256] = {0};
GetUICheatInfo(hwndDlg, namebuf, &a, &v, &c);
name = namebuf;
FCEUI_SetCheat(selcheat, name, a, v, c, -1, 1);
FCEUI_GetCheat(selcheat, NULL, &a, &v, &c, &s, NULL);
RedoCheatsCallB(name, a, v, c, s, 1, &selcheat);
FCEUI_SetCheat(selcheat, &name, a, v, c, -1, 1);
FCEUI_GetCheat(selcheat, nullptr, &a, &v, &c, &s, NULL);
RedoCheatsCallB(name.c_str(), a, v, c, s, 1, &selcheat);
SendDlgItemMessage(hwndDlg, IDC_LIST_CHEATS, LVM_SETSELECTIONMARK, 0, selcheat);
SetDlgItemText(hwndDlg, IDC_CHEAT_ADDR, (LPCSTR)U16ToStr(a));
@ -902,9 +905,9 @@ INT_PTR CALLBACK CheatConsoleCallB(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARA
selcheat = pNMListView->iItem;
if (selcheat >= 0)
{
char* name = ""; uint32 a; uint8 v; int s; int c;
std::string name; uint32 a; uint8 v; int s; int c;
FCEUI_GetCheat(selcheat, &name, &a, &v, &c, &s, NULL);
SetDlgItemText(hwndDlg, IDC_CHEAT_NAME, (LPCSTR)name);
SetDlgItemText(hwndDlg, IDC_CHEAT_NAME, (LPCSTR)name.c_str());
SetDlgItemText(hwndDlg, IDC_CHEAT_ADDR, (LPCSTR)U16ToStr(a));
SetDlgItemText(hwndDlg, IDC_CHEAT_VAL, (LPCSTR)U8ToStr(v));
SetDlgItemText(hwndDlg, IDC_CHEAT_COM, (LPCSTR)(c == -1 ? "" : U8ToStr(c)));
@ -930,11 +933,11 @@ INT_PTR CALLBACK CheatConsoleCallB(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARA
pNMListView->uNewState & INDEXTOSTATEIMAGEMASK(2))
{
int tmpsel = pNMListView->iItem;
char* name = ""; int s;
std::string name; int s;
FCEUI_GetCheat(tmpsel, &name, NULL, NULL, NULL, &s, NULL);
if (!s)
{
FCEUI_SetCheat(tmpsel, name, -1, -1, -2, s ^= 1, 1);
FCEUI_SetCheat(tmpsel, &name, -1, -1, -2, s ^= 1, 1);
UpdateCheatRelatedWindow();
UpdateCheatListGroupBoxUI();
}
@ -944,11 +947,11 @@ INT_PTR CALLBACK CheatConsoleCallB(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARA
pNMListView->uNewState & INDEXTOSTATEIMAGEMASK(1))
{
int tmpsel = pNMListView->iItem;
char* name = ""; int s;
std::string name; int s;
FCEUI_GetCheat(tmpsel, &name, NULL, NULL, NULL, &s, NULL);
if (s)
{
FCEUI_SetCheat(tmpsel, name, -1, -1, -2, s ^= 1, 1);
FCEUI_SetCheat(tmpsel, &name, -1, -1, -2, s ^= 1, 1);
UpdateCheatRelatedWindow();
UpdateCheatListGroupBoxUI();

View File

@ -822,7 +822,7 @@ void UpdateColorTable()
int addrtodelete; // This is a very ugly hackish method of doing this
int cheatwasdeleted; // but it works and that is all that matters here.
int DeleteCheatCallB(char *name, uint32 a, uint8 v, int compare,int s,int type, void *data){ //mbg merge 6/29/06 - added arg
int DeleteCheatCallB(const char *name, uint32 a, uint8 v, int compare,int s,int type, void *data){ //mbg merge 6/29/06 - added arg
if(cheatwasdeleted == -1)return 1;
cheatwasdeleted++;
if(a == addrtodelete){
@ -901,7 +901,7 @@ void UnfreezeAllRam() {
int i=0;
char * Cname;
std::string Cname;
uint32 Caddr;
int Ctype;
@ -919,7 +919,7 @@ void UnfreezeAllRam() {
// would be added by the freeze command. Manual unfreeze should let them
// make that mistake once or twice, in case they like it that way.
FCEUI_GetCheat(i,&Cname,&Caddr,NULL,NULL,NULL,&Ctype);
if ((Cname[0] == '\0') && ((Caddr < 0x2000) || ((Caddr >= 0x6000) && (Caddr < 0x8000))) && (Ctype == 1)) {
if ((Cname.empty()) && ((Caddr < 0x2000) || ((Caddr >= 0x6000) && (Caddr < 0x8000))) && (Ctype == 1)) {
// Already Added, so consider it a success
FreezeRam(Caddr,-1,1);

View File

@ -773,7 +773,7 @@ static int emu_delgamegenie(lua_State *L) {
int GGaddr, GGcomp, GGval;
uint32 i=0;
char * Cname;
std::string Cname;
uint32 Caddr;
uint8 Cval;
int Ccompare, Ctype;
@ -786,7 +786,7 @@ static int emu_delgamegenie(lua_State *L) {
while (FCEUI_GetCheat(i,&Cname,&Caddr,&Cval,&Ccompare,NULL,&Ctype)) {
if ((!strcmp(msg,Cname)) && (GGaddr == Caddr) && (GGval == Cval) && (GGcomp == Ccompare) && (Ctype == 1)) {
if ((Cname == msg) && (GGaddr == Caddr) && (GGval == Cval) && (GGcomp == Ccompare) && (Ctype == 1)) {
// Delete cheat code
if (FCEUI_DelCheat(i)) {
lua_pushboolean(L, true);