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

Buffer overflow in SeBa.C, paramlog string #24

Open
evertrol opened this issue May 1, 2024 · 1 comment
Open

Buffer overflow in SeBa.C, paramlog string #24

evertrol opened this issue May 1, 2024 · 1 comment

Comments

@evertrol
Copy link

evertrol commented May 1, 2024

The paramlog string is set to contain 90 characters (line 261). But the formatting string on line 353 is about 120 characters long; well over those 90 characters. This can cause a buffer overflow.

Previous versions didn't have the CT_method section of the formatting string, making it shorter (possibly just about 90 characters, depending on the floating point values having 1 or multiple digits on the left side of the decimal dot), so the problem will likely not have shown. This part of the string was introduced in a recent commit, bbd9187.

In addition, certainly in non-optimised compiled code, strings often have some leeway that they can run over their allocated buffer (unintentionally, and basically undefined behaviour), so this may also cause some people to not have come across this error yet (for example, clang 15 on macOS happily runs SeBa, while gcc 11 on Ubuntu 22.04 will throw a buffer overflow error).

One fix is to increase the buffer size, to e.g. 150. In addition, it might be good to use snprintf instead of sprintf, which makes its intention clearer to a future programmer that extends the formatting string. Given that this is C++ code, it may also be useful to use std::string instead, but that probably requires a C++11 flag for the compiler and restructuring more code.

Note that seedlog does not have (yet) this problem, eyeballing the length of the formatting string in line 376.

Here's the actual error (gcc 11 on Ubuntu 22.04):

$ ./SeBa -M 2 -m 1 -e 0.2 -a 200 -T 13500 -z 0.001
random number generator seed = 1714574797
*** buffer overflow detected ***: terminated
zsh: IOT instruction (core dumped)  ./SeBa -M 2 -m 1 -e 0.2 -a 200 -T 13500 -z 0.001
@rieder
Copy link
Member

rieder commented May 1, 2024

I'd just increase the buffer size in this case.

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

No branches or pull requests

2 participants