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.

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

[raylib] Buffer Overflow In Initialization #4695

Closed
SNAIDY1 opened this issue Jan 17, 2025 · 7 comments
Closed

[raylib] Buffer Overflow In Initialization #4695

SNAIDY1 opened this issue Jan 17, 2025 · 7 comments

Comments

@SNAIDY1
Copy link

SNAIDY1 commented Jan 17, 2025

I couldn't find anyone with this problem And i tried to see any solution but nothing.
And this issue is not only in raylib, even in raygui.

./program

INFO: Initializing raylib 5.5
INFO: Platform backend: DESKTOP (GLFW)
INFO: Supported raylib modules:
INFO: > rcore:..... loaded (mandatory)
INFO: > rlgl:...... loaded (mandatory)
INFO: > rshapes:... loaded (optional)
INFO: > rtextures:. loaded (optional)
INFO: > rtext:..... loaded (optional)
INFO: > rmodels:... loaded (optional)
INFO: > raudio:.... loaded (optional)
INFO: DISPLAY: Device initialized successfully
INFO: > Display size: 1920 x 1080
INFO: > Screen size: 800 x 450
INFO: > Render size: 800 x 450
INFO: > Viewport offsets: 0, 0
INFO: GLAD: OpenGL extensions loaded successfully
INFO: GL: Supported extensions count: 401
INFO: GL: OpenGL device information:
INFO: > Vendor: NVIDIA Corporation
INFO: > Renderer: NVIDIA GeForce RTX 3050/PCIe/SSE2
INFO: > Version: 3.3.0 NVIDIA 565.77
INFO: > GLSL: 3.30 NVIDIA via Cg compiler
INFO: GL: VAO extension detected, VAO functions loaded successfully
INFO: GL: NPOT textures extension detected, full NPOT textures supported
INFO: GL: DXT compressed textures supported
INFO: GL: ETC2/EAC compressed textures supported
*** buffer overflow detected ***: terminated
Aborted (core dumped)

And This is The Code :

#include "raylib.h"

int main() {
    InitWindow(800, 450, "Minimal Test");
    SetTargetFPS(60);

    while (!WindowShouldClose()) {
        BeginDrawing();
        ClearBackground(RAYWHITE);
        DrawText("Hello, Ray!", 190, 200, 20, LIGHTGRAY);
        EndDrawing();
    }

    CloseWindow();
    return 0;
}
@SNAIDY1
Copy link
Author

SNAIDY1 commented Jan 17, 2025

1. Which OS? : Arch Linux
2. Did you test with current master branch?:No
3. Did you set any specific `GRAPHICS`?:No
4. How are you compiling it?:gcc program.c -o game -lraylib (I Tested All Ways)

@WyntrHeart
Copy link

WyntrHeart commented Jan 17, 2025

I had this same issue, ran a backtrace with gdb:

#1  __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
#2  __GI___pthread_kill (threadid=<optimized out>, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3  0x00007ffff7c4526e in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4  0x00007ffff7c288ff in __GI_abort () at ./stdlib/abort.c:79
#5  0x00007ffff7c297b6 in __libc_message_impl (fmt=fmt@entry=0x7ffff7dce765 "*** %s ***: terminated\n")
    at ../sysdeps/posix/libc_fatal.c:132
#6  0x00007ffff7d36c19 in __GI___fortify_fail (msg=msg@entry=0x7ffff7dce74c "buffer overflow detected")
    at ./debug/fortify_fail.c:24
#7  0x00007ffff7d365d4 in __GI___chk_fail () at ./debug/chk_fail.c:28
#8  0x00007ffff7d37fd5 in __strcpy_chk (dest=0x555555630620 <CORE+2272> " Motion Sensors", 
    src=0x55555564238c <_glfw+26636> "Sony Interactive Entertainment DUALSHOCK®4 USB Wireless Adaptor Touchpad", 
    destlen=64) at ./debug/strcpy_chk.c:30
#9  0x0000555555584324 in InitPlatform ()
#10 0x000055555558462e in InitWindow ()
#11 0x000055555555d0ef in main () at minimal.c:4

So I tried removing the DS4 wireless adapter and sure enough, crash went away. The device name is 74 bytes long, while the field it's entered into is only 64 bytes long. Should be an easy fix, but I'm a noob and have no idea how to upload the changes myself tbh

@SNAIDY1
Copy link
Author

SNAIDY1 commented Jan 17, 2025

thank you asdqwe , it works now!!

@WyntrHeart
Copy link

I found the relevant line of code in rcore.c, doubled the buffer size from 64B to 128B and it works. Here's the diff output, sorry I don't know how to contribute via git

diff --git a/src/rcore.c b/src/rcore.c
index a90a603..3e105a6 100644
--- a/src/rcore.c
+++ b/src/rcore.c
@@ -352,7 +352,7 @@ typedef struct CoreData {
             int lastButtonPressed;          // Register last gamepad button pressed
             int axisCount[MAX_GAMEPADS];    // Register number of available gamepad axis
             bool ready[MAX_GAMEPADS];       // Flag to know if gamepad is ready
-            char name[MAX_GAMEPADS][64];    // Gamepad name holder
+            char name[MAX_GAMEPADS][128];    // Gamepad name holder
             char currentButtonState[MAX_GAMEPADS][MAX_GAMEPAD_BUTTONS];     // Current gamepad buttons state
             char previousButtonState[MAX_GAMEPADS][MAX_GAMEPAD_BUTTONS];    // Previous gamepad buttons state
             float axisState[MAX_GAMEPADS][MAX_GAMEPAD_AXIS];                // Gamepad axis state

@hanaxars
Copy link
Contributor

Ok. I did dive a little deeper, and some more changes needed.
Unfortunately there are some hardcoded numbers in rcore_desktop_glfw.c and rcore_desktop_sdl.c .

In rcore_desktop_sdl.c first 63 bytes are copied and then null teminated:

strncpy(CORE.Input.Gamepad.name[jid], SDL_GameControllerNameForIndex(jid), 63);
CORE.Input.Gamepad.name[jid][63] = '\0';

strncpy(CORE.Input.Gamepad.name[i], SDL_GameControllerNameForIndex(i), 63);
CORE.Input.Gamepad.name[i][63] = '\0';

...and then when disconnected 64 bytes are zeroed.
CORE.Input.Gamepad.ready[jid] = false;
memset(CORE.Input.Gamepad.name[jid], 0, 64);

In rcore_desktop_glfw.c it's a little bit adventurous, name is copied with strcpy without null terminating, or length checking.

if (glfwJoystickPresent(i)) strcpy(CORE.Input.Gamepad.name[i], glfwGetJoystickName(i));

strcpy(CORE.Input.Gamepad.name[jid], glfwGetJoystickName(jid));

...and again when disconnected 64 bytes are zeroed.
memset(CORE.Input.Gamepad.name[jid], 0, 64);

Honestly, @raysan5 I don't know what's the best approach here. Should we define something like MAX_NAME_LENGTH and then strncpy some number of chars and null terminate them?

@raysan5
Copy link
Owner

raysan5 commented Jan 17, 2025

@WyntrHeart Wow! Very good catch! Gamepad name retrieval was added lately...

Honestly, @raysan5 I don't know what's the best approach here. Should we define something like MAX_NAME_LENGTH and then strncpy some number of chars and null terminate them?

Yeah, it seems the best solution. Feel free to send a PR!

@hanaxars
Copy link
Contributor

hanaxars commented Jan 17, 2025

@raysan5 Should I close #4696?

I think rcore_desktop_sdl.c needs same changes.
And also both files have this line: memset(CORE.Input.Gamepad.name[jid], 0, 64);

raysan5 added a commit that referenced this issue Jan 18, 2025
@raysan5 raysan5 closed this as completed Jan 18, 2025
ngynkvn pushed a commit to ngynkvn/raylib that referenced this issue Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants