brokenphilip / OMSIPresence

Discord Rich Presence plugin for OMSI 2
4 stars 0 forks source link

Crash when writing to an undersize buffer & Bus line too long #3

Closed brokenphilip closed 1 year ago

brokenphilip commented 1 year ago

When writing to a character array buffer smaller than what was passed to it through sprintf_s, an invalid parameter exception is thrown, causing the game to crash. devenv_2023_October_17_152235

In particular, the crash I was sent occurred on line 204 of discord.cpp, as shown here: https://github.com/brokenphilip/OMSIPresence/blob/fd6cd2e32f529a92ef3ce05e19cc5391e48be553/OMSIPresence/src/discord.cpp#L194-L211

This crash bypassed the vectored exception handler, directly leading to Windows Error Reporting taking over and creating the dump in AppData instead. This information will be added to the README of the project.

I have been using sprintf_s in my projects for months (years if including projects outside GitHub), and I have never gotten a crash this trivial - I was under the false impression that sprintf_s would correctly truncate the string if it's too long.

The first part of the solution is to replace sprintf_s with one that correctly truncates the strings if they're too big to fit. For this purpose, I have chosen _snprintf_s for this purpose. The second part of the solution is increasing the maximum size of the line text - which I have doubled (from 8 to 16). As a precautionary step, I have also increased the maximum sizes of the terminus and next bus stop texts by double their size (from 32 to 64), taking into consideration that, for example, public transport in Belgrade has bus stations which are longer than 32 characters: image

This issue has been resolved, but will stay open should additional complaints arise. It will be pushed in the next 1.1 update as soon as I get confirmation from the reporter that this fix worked for them (or sooner, depending on the response time).

If you are unsure whether this is your crash, feel free to create a new issue - you will be referenced to this one if it is.

brokenphilip commented 1 year ago

Fixed as of 1.1.