Fix: Use after return for stack allocated object #7926
Conversation
GetCurrentSong() returns a copy of an object containing a string buffer. A pointer to this buffer can get stored into the global string formatting parameters, in which it can escape the function that called GetCurrentSong(). This means a pointer to an invalid object is stuck in a global variable. Solve it by returning a reference instead, and make a fixed no-song object that a reference to will stay valid.
This reverts commit f8a5da5.
Pointer to string buffer on the stack could escape through the global string parameter buffer. Clear out the parameter after drawing, before return.
|
Made another, shorter fix, that can theoretically still let a stack object pointer escape, but in practice shouldn't. (I don't think we have more than one thread accessing the string formatting data.) Really, I think this problem is entirely theoretical. While a pointer to a stack object does escape and become invalid, it should never be accessed, unless code somewhere else forgets to fill out its string parameters. |
|
No, if the pointer escapes but is never accessed it doesn't matter, but there is different issue. As GetCurrentSong returns by copy, struct returned by it becomes invalid as soon as SetDParamStr returns. But SetDParamStr stores pointer which is used inside DrawString below, at which point this pointer is invalid. This is alternative fix: |
|
Let's use #7928 instead |
GetCurrentSong() returns a copy of an object containing a string buffer. A pointer to this buffer can get stored into the global string formatting parameters, in which it can escape the function that called GetCurrentSong(). This means a pointer to an invalid object is stuck in a global variable. Solve it by returning a reference instead, and make a fixed no-song object that a reference to will stay valid.