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

Fix fann_error buffer overflow #2

Merged
merged 4 commits into from
Aug 16, 2014

Conversation

bukka
Copy link
Member

@bukka bukka commented Dec 11, 2013

There was a bug in fann_error function. The errstr string was limited by FANN_ERRSTR_MAX (128) characters. It caused buffer overflow for all error messages with longer file path as a parameter (FANN_E_CANT_READ_CONFIG, FANN_E_CANT_OPEN_CONFIG_R...). It obviously resulted in the heap corruption...

In addition there was a memory leak for errstr when errdat was NULL.

This patch fixes both bugs. I checked it with valgrind and also run all my test tests that I have in my php fann ext.

@bukka
Copy link
Member Author

bukka commented Dec 11, 2013

Please double check that PATH_MAX definition for Windows is ok: https://github.com/libfann/fann/pull/2/files#diff-f413e9a581932c55382bb9a96c24fea3R35

I don't have win so I could not test it...

@remicollet
Copy link

In addition to those changes, I think the vsprintf / sprintf unsecure call should be avoid and replace by vsnprintf / snprintf.

steffennissen added a commit that referenced this pull request Aug 16, 2014
@steffennissen steffennissen merged commit 4489af1 into libfann:master Aug 16, 2014
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 this pull request may close these issues.

4 participants