Skip to content

Commit b0e93e4

Browse files
committed
Prevent crashes if freed objects are passed to SDL API functions
Instead of using the magic tag in the object, we'll actually keep track of valid objects Fixes #9869 Fixes #9235
1 parent 57a1593 commit b0e93e4

28 files changed

+191
-126
lines changed

src/SDL.c

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,7 @@ void SDL_Quit(void)
543543
SDL_DBus_Quit();
544544
#endif
545545

546+
SDL_SetObjectsInvalid();
546547
SDL_ClearHints();
547548
SDL_AssertionsQuit();
548549

@@ -563,20 +564,6 @@ void SDL_Quit(void)
563564
SDL_bInMainQuit = SDL_FALSE;
564565
}
565566

566-
/* Assume we can wrap SDL_AtomicInt values and cast to Uint32 */
567-
SDL_COMPILE_TIME_ASSERT(sizeof_object_id, sizeof(int) == sizeof(Uint32));
568-
569-
Uint32 SDL_GetNextObjectID(void)
570-
{
571-
static SDL_AtomicInt last_id;
572-
573-
Uint32 id = (Uint32)SDL_AtomicIncRef(&last_id) + 1;
574-
if (id == 0) {
575-
id = (Uint32)SDL_AtomicIncRef(&last_id) + 1;
576-
}
577-
return id;
578-
}
579-
580567
/* Get the library version number */
581568
int SDL_GetVersion(void)
582569
{

src/SDL_hashtable.c

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ SDL_bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const vo
8383
SDL_HashItem *item;
8484
const Uint32 hash = calc_hash(table, key);
8585

86+
if (!table) {
87+
return SDL_FALSE;
88+
}
89+
8690
if ( (!table->stackable) && (SDL_FindInHashTable(table, key, NULL)) ) {
8791
return SDL_FALSE;
8892
}
@@ -107,6 +111,10 @@ SDL_bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, const
107111
void *data = table->data;
108112
SDL_HashItem *i;
109113

114+
if (!table) {
115+
return SDL_FALSE;
116+
}
117+
110118
for (i = table->table[hash]; i; i = i->next) {
111119
if (table->keymatch(key, i->key, data)) {
112120
if (_value) {
@@ -126,6 +134,10 @@ SDL_bool SDL_RemoveFromHashTable(SDL_HashTable *table, const void *key)
126134
SDL_HashItem *prev = NULL;
127135
void *data = table->data;
128136

137+
if (!table) {
138+
return SDL_FALSE;
139+
}
140+
129141
for (item = table->table[hash]; item; item = item->next) {
130142
if (table->keymatch(key, item->key, data)) {
131143
if (prev) {
@@ -134,7 +146,9 @@ SDL_bool SDL_RemoveFromHashTable(SDL_HashTable *table, const void *key)
134146
table->table[hash] = item->next;
135147
}
136148

137-
table->nuke(item->key, item->value, data);
149+
if (table->nuke) {
150+
table->nuke(item->key, item->value, data);
151+
}
138152
SDL_free(item);
139153
return SDL_TRUE;
140154
}
@@ -149,6 +163,10 @@ SDL_bool SDL_IterateHashTableKey(const SDL_HashTable *table, const void *key, co
149163
{
150164
SDL_HashItem *item = *iter ? ((SDL_HashItem *) *iter)->next : table->table[calc_hash(table, key)];
151165

166+
if (!table) {
167+
return SDL_FALSE;
168+
}
169+
152170
while (item) {
153171
if (table->keymatch(key, item->key, table->data)) {
154172
*_value = item->value;
@@ -169,6 +187,10 @@ SDL_bool SDL_IterateHashTable(const SDL_HashTable *table, const void **_key, con
169187
SDL_HashItem *item = (SDL_HashItem *) *iter;
170188
Uint32 idx = 0;
171189

190+
if (!table) {
191+
return SDL_FALSE;
192+
}
193+
172194
if (item) {
173195
const SDL_HashItem *orig = item;
174196
item = item->next;
@@ -219,7 +241,9 @@ void SDL_DestroyHashTable(SDL_HashTable *table)
219241
SDL_HashItem *item = table->table[i];
220242
while (item) {
221243
SDL_HashItem *next = item->next;
222-
table->nuke(item->key, item->value, data);
244+
if (table->nuke) {
245+
table->nuke(item->key, item->value, data);
246+
}
223247
SDL_free(item);
224248
item = next;
225249
}

src/SDL_internal.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,8 @@
279279
#define SDL_MAIN_NOIMPL /* don't drag in header-only implementation of SDL_main */
280280
#include <SDL3/SDL_main.h>
281281

282+
#include "SDL_utils_c.h"
283+
282284
/* The internal implementations of these functions have up to nanosecond precision.
283285
We can expose these functions as part of the API if we want to later.
284286
*/
@@ -287,7 +289,6 @@
287289
extern "C" {
288290
#endif
289291

290-
extern Uint32 SDLCALL SDL_GetNextObjectID(void);
291292
extern int SDLCALL SDL_WaitSemaphoreTimeoutNS(SDL_Semaphore *sem, Sint64 timeoutNS);
292293
extern int SDLCALL SDL_WaitConditionTimeoutNS(SDL_Condition *cond, SDL_Mutex *mutex, Sint64 timeoutNS);
293294
extern SDL_bool SDLCALL SDL_WaitEventTimeoutNS(SDL_Event *event, Sint64 timeoutNS);

src/SDL_utils.c

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
*/
2121
#include "SDL_internal.h"
2222

23-
#include "SDL_utils_c.h"
23+
#include "SDL_hashtable.h"
2424

2525
/* Common utility functions that aren't in the public API */
2626

@@ -100,3 +100,68 @@ SDL_bool SDL_endswith(const char *string, const char *suffix)
100100
}
101101
return SDL_FALSE;
102102
}
103+
104+
/* Assume we can wrap SDL_AtomicInt values and cast to Uint32 */
105+
SDL_COMPILE_TIME_ASSERT(sizeof_object_id, sizeof(int) == sizeof(Uint32));
106+
107+
Uint32 SDL_GetNextObjectID(void)
108+
{
109+
static SDL_AtomicInt last_id;
110+
111+
Uint32 id = (Uint32)SDL_AtomicIncRef(&last_id) + 1;
112+
if (id == 0) {
113+
id = (Uint32)SDL_AtomicIncRef(&last_id) + 1;
114+
}
115+
return id;
116+
}
117+
118+
static SDL_HashTable *SDL_objects;
119+
120+
static Uint32 SDL_HashObject(const void *key, void *unused)
121+
{
122+
return (Uint32)(uintptr_t)key;
123+
}
124+
125+
static SDL_bool SDL_KeyMatchObject(const void *a, const void *b, void *unused)
126+
{
127+
return (a == b);
128+
}
129+
130+
void SDL_SetObjectValid(void *object, SDL_ObjectType type, SDL_bool valid)
131+
{
132+
SDL_assert(object != NULL);
133+
134+
if (valid) {
135+
if (!SDL_objects) {
136+
SDL_objects = SDL_CreateHashTable(NULL, 32, SDL_HashObject, SDL_KeyMatchObject, NULL, SDL_FALSE);
137+
}
138+
139+
SDL_InsertIntoHashTable(SDL_objects, object, (void *)(uintptr_t)type);
140+
} else {
141+
if (SDL_objects) {
142+
SDL_RemoveFromHashTable(SDL_objects, object);
143+
}
144+
}
145+
}
146+
147+
SDL_bool SDL_ObjectValid(void *object, SDL_ObjectType type)
148+
{
149+
if (!object) {
150+
return SDL_FALSE;
151+
}
152+
153+
const void *object_type;
154+
if (!SDL_FindInHashTable(SDL_objects, object, &object_type)) {
155+
return SDL_FALSE;
156+
}
157+
158+
return (((SDL_ObjectType)(uintptr_t)object_type) == type);
159+
}
160+
161+
void SDL_SetObjectsInvalid(void)
162+
{
163+
if (SDL_objects) {
164+
SDL_DestroyHashTable(SDL_objects);
165+
SDL_objects = NULL;
166+
}
167+
}

src/SDL_utils_c.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,24 @@ extern void SDL_CalculateFraction(float x, int *numerator, int *denominator);
3232

3333
extern SDL_bool SDL_endswith(const char *string, const char *suffix);
3434

35+
typedef enum
36+
{
37+
SDL_OBJECT_TYPE_UNKNOWN,
38+
SDL_OBJECT_TYPE_WINDOW,
39+
SDL_OBJECT_TYPE_RENDERER,
40+
SDL_OBJECT_TYPE_TEXTURE,
41+
SDL_OBJECT_TYPE_JOYSTICK,
42+
SDL_OBJECT_TYPE_GAMEPAD,
43+
SDL_OBJECT_TYPE_HAPTIC,
44+
SDL_OBJECT_TYPE_SENSOR,
45+
SDL_OBJECT_TYPE_HIDAPI_DEVICE,
46+
SDL_OBJECT_TYPE_HIDAPI_JOYSTICK,
47+
48+
} SDL_ObjectType;
49+
50+
extern Uint32 SDL_GetNextObjectID(void);
51+
extern void SDL_SetObjectValid(void *object, SDL_ObjectType type, SDL_bool valid);
52+
extern SDL_bool SDL_ObjectValid(void *object, SDL_ObjectType type);
53+
extern void SDL_SetObjectsInvalid(void);
54+
3555
#endif /* SDL_utils_h_ */

src/audio/SDL_audio.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
#include "SDL_audio_c.h"
2424
#include "SDL_sysaudio.h"
2525
#include "../thread/SDL_systhread.h"
26-
#include "../SDL_utils_c.h"
2726

2827
// Available audio drivers
2928
static const AudioBootStrap *const bootstrap[] = {

src/audio/dsp/SDL_dspaudio.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
#include <sys/soundcard.h>
3838

3939
#include "../SDL_audiodev_c.h"
40-
#include "../../SDL_utils_c.h"
4140
#include "SDL_dspaudio.h"
4241

4342
static void DSP_DetectDevices(SDL_AudioDevice **default_output, SDL_AudioDevice **default_capture)

src/events/SDL_keyboard.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,7 @@ int SDL_SetKeyboardFocus(SDL_Window *window)
891891
SDL_Keyboard *keyboard = &SDL_keyboard;
892892

893893
if (window) {
894-
if (!video || window->magic != &video->window_magic || window->is_destroying) {
894+
if (!SDL_ObjectValid(window, SDL_OBJECT_TYPE_WINDOW) || window->is_destroying) {
895895
return SDL_SetError("Invalid window");
896896
}
897897
}

src/haptic/SDL_haptic.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,9 @@
2525
#include "../joystick/SDL_joystick_c.h" /* For SDL_IsJoystickValid */
2626

2727
static SDL_Haptic *SDL_haptics = NULL;
28-
static char SDL_haptic_magic;
2928

3029
#define CHECK_HAPTIC_MAGIC(haptic, retval) \
31-
if (!haptic || haptic->magic != &SDL_haptic_magic) { \
30+
if (!SDL_ObjectValid(haptic, SDL_OBJECT_TYPE_HAPTIC)) { \
3231
SDL_InvalidParamError("haptic"); \
3332
return retval; \
3433
}
@@ -135,7 +134,7 @@ SDL_Haptic *SDL_OpenHaptic(SDL_HapticID instance_id)
135134
}
136135

137136
/* Initialize the haptic device */
138-
haptic->magic = &SDL_haptic_magic;
137+
SDL_SetObjectValid(haptic, SDL_OBJECT_TYPE_HAPTIC, SDL_TRUE);
139138
haptic->instance_id = instance_id;
140139
haptic->rumble_id = -1;
141140
if (SDL_SYS_HapticOpen(haptic) < 0) {
@@ -318,7 +317,7 @@ void SDL_CloseHaptic(SDL_Haptic *haptic)
318317
}
319318
}
320319
SDL_SYS_HapticClose(haptic);
321-
haptic->magic = NULL;
320+
SDL_SetObjectValid(haptic, SDL_OBJECT_TYPE_HAPTIC, SDL_FALSE);
322321

323322
/* Remove from the list */
324323
hapticlist = SDL_haptics;

src/haptic/SDL_syshaptic.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@ struct haptic_effect
4040
*/
4141
struct SDL_Haptic
4242
{
43-
const void *magic;
44-
4543
SDL_HapticID instance_id; /* Device instance, monotonically increasing from 0 */
4644
char *name; /* Device name - system dependent */
4745

0 commit comments

Comments
 (0)