mas-bandwidth / yojimbo

A network library for client/server games written in C++
BSD 3-Clause "New" or "Revised" License
2.45k stars 238 forks source link

Fix for YOJIMBO_NEW() null pointer dereference. #172

Closed sherief closed 2 years ago

sherief commented 2 years ago

YOJIMBO_DECLARE_MESSAGE_TYPE() expands to:

message = YOJIMBO_NEW( allocator, msg_action );
    if ( !message )
        return NULL;

But YOJIMBO_NEW() never checks the result from Allocate(), it just immediately passes it to placement new(), which dereferences a null pointer if Allocate() fails. This makes it impossible to gracefully handle OOM situations, and leads to unpreventable crashes.

This change replaces the existing macro with a short lambda evaluated on the spot that checks for the null pointer and avoids placement new in OOM situations.

sherief commented 2 years ago

The macOS build is failing but I think this should be fixable by updating the C++ standard used by clang++ while compiling (and I doubt any shipping and supported macOS / iOS product won't support this lambda), but I'm a bit lost in the build files and I don't have a Mac to test on - if you'd point me to where to make the changes I'll update the pull request.

Thanks a lot Glenn!

gafferongames commented 2 years ago

Hi, I'd really like to keep the C++ standard compatible with older versions. I don't want to require Modern C++ to use yojimbo. Can somebody please apply a fix that works with older versions of C++ standard?

sherief commented 2 years ago

No worries, just let me know what version of the C++ standard you want to target and I'll investigate the best way to provide a fix that's in line with that requirement.

gafferongames commented 2 years ago

C++ 03

grahamreeds commented 2 years ago

Really? Are there any build targets that don't support C++11? Even the embedded compilers I use - which are notoriously slow at updating their standards - support 11 now.

GR

Sent from my Pixel C

On Tue, 15 Feb 2022, 03:44 Glenn Fiedler, @.***> wrote:

C++ 03

— Reply to this email directly, view it on GitHub https://github.com/networkprotocol/yojimbo/pull/172#issuecomment-1039822818, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGL3HFEGOW65DCDPJHZZM3U3HD2XANCNFSM5NAGUOHQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

gafferongames commented 2 years ago

My man, I don't like Modern C++