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

Memory leak vulnerability in try-catch in walletconsole/Util.cpp #1286

Closed
brutalsavage opened this issue Feb 5, 2021 · 3 comments · Fixed by #1291
Closed

Memory leak vulnerability in try-catch in walletconsole/Util.cpp #1286

brutalsavage opened this issue Feb 5, 2021 · 3 comments · Fixed by #1291
Assignees
Labels
bug Something isn't working

Comments

@brutalsavage
Copy link

brutalsavage commented Feb 5, 2021

Describe the bug
buffer is allocated in try block however there is no delete call for deallocation in catch block for if the exception is thrown after the buffer allocation. If the exception is thrown after new there is a memory leak. Potential exception can be thrown from infile.gcount() or infile.close() .

This is similar to #921 except the missing delete is in the catch block.

@brutalsavage brutalsavage added the bug Something isn't working label Feb 5, 2021
@optout21
Copy link
Contributor

optout21 commented Feb 6, 2021

@brutalsavage , could you specify the source file location? thanks

@brutalsavage
Copy link
Author

brutalsavage commented Feb 7, 2021

@catenocrypt here is the vulnerable function

bool Util::fileR(const string& fileName, string& res) {
if (!fileExists(fileName)) {
_out << "Error: File not found '" << fileName << "'" << endl;
return false;
}
try {
ifstream infile(fileName, std::ios::in | std::ios::binary);
// get length of file:
infile.seekg (0, infile.end);
auto length = infile.tellg();
infile.seekg (0, infile.beg);
char* buffer = new char[length];
if (!infile.read(buffer, length)) {
_out << "Could not read file '" << fileName << "'" << endl;
delete[] buffer;
return false;
}
auto red = infile.gcount();
infile.close();
res = string(TW::hex(data((const byte*)buffer, red)));
delete[] buffer;
_out << "Read " << red << " bytes from file '" << fileName << "'." << endl;
return true;
} catch (exception& ex) {
_out << "Error reading from file '" << fileName << "': " << ex.what() << endl;
return false;
}
}

@brutalsavage
Copy link
Author

@catenocrypt Hi! Thanks for taking a look at this and fixing it. This vulnerability was detected by our deep learning based vulnerability detection model. Along with the detection, our model also localizes the vulnerability by producing a version of the function with code that contributed to the vulnerability highlighted. We provide the localization output of the vulnerable function identified in this Issue. The intensity of the highlight correspond to how important the code snippet was for vulnerability detection in this function.

As part of our university research project we would like to evaluate the usefulness of the model’s localization outputs. You can help us out by clicking one of the options below:

  • This vulnerability is a real vulnerability and the localization output is useful. check
  • This vulnerability is a real vulnerability but the localization output is not useful. check
  • This vulnerability is not a real vulnerability. check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants