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

[Bug]: Use after free via SetDParamStr for STR_CONFIG_ERROR_INVALID_GRF #10129

Closed
JGRennison opened this issue Nov 3, 2022 · 0 comments · Fixed by #10130
Closed

[Bug]: Use after free via SetDParamStr for STR_CONFIG_ERROR_INVALID_GRF #10129

JGRennison opened this issue Nov 3, 2022 · 0 comments · Fixed by #10130

Comments

@JGRennison
Copy link
Contributor

Version of OpenTTD

master

Expected result

No use after frees.

Actual result

SetDParamStr(0, StrEmpty(filename) ? item->name : filename); at https://github.com/OpenTTD/OpenTTD/blob/master/src/settings.cpp#L1019

In this case item->name is a std::string, file is a const char *.
The ternary's result type is std::string, so a temporary std::string is created from filename.
This goes out of scope before it is accessed from ShowErrorMessage on the next line.

While this particular one is trivial to fix, probably there are more like this in the codebase. I found it by chance when testing something else.

void SetDParamStr(uint n, const std::string &str) seems a somewhat problematic function signature in general as it can be difficult to avoid passing a temporary/rvalue.

Steps to reproduce

Use AddressSanitizer or some other heap checker, and trigger this code path using the NewGRF config window.

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

Successfully merging a pull request may close this issue.

1 participant