ValveSoftware / halflife

Half-Life 1 engine based games
Other
3.6k stars 598 forks source link

Out of bounds access in CStudioModelRenderer::StudioSetupBones #3360

Open SamVanheer opened 11 months ago

SamVanheer commented 11 months ago

The studio model renderer accesses bone data using an invalid index here: https://github.com/ValveSoftware/halflife/blob/c7240b965743a53a29491dd49320c88eecf6257b/cl_dll/StudioModelRenderer.cpp#L941

Bones that don't have a parent have a parent index of -1. This code thus treats the memory region right before the first bone to be a bone as well. Since the entire model is loaded into a contiguous chunk of memory this is accessing another part of the model and reinterpreting it. This is also why it doesn't crash due to accessing invalid memory.

It is also possible to access out of bounds memory if the parent index is invalid.

This can be fixed by adding bounds checking to that code. Additionally bounds checking in the model loading code can help catch invalid access as well.

tschumann commented 11 months ago

So it should just be changed to: else if ( pbones[i].parent != -1 && !strcmp( pbones[ pbones[i].parent ].name, "Bip01 Pelvis" ) ) Or is there more to it?

SamVanheer commented 11 months ago

You can see how i fixed here: https://github.com/SamVanheer/halflife-updated/commit/68bb362ffb0d096d479c3bbca36f3777a9a5b2db