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

Add a JSON key indicating whether we encountered a short load config #14

Open
woodruffw opened this issue Aug 31, 2018 · 2 comments
Open
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest
Milestone

Comments

@woodruffw
Copy link
Member

Right now, we just spit out some warning messages if the IMAGE_LOAD_CONFIG_DIRECTORY doesn't exist or is too short to perform some checks, but it'd be nice to include that in the JSON output as well.

@woodruffw woodruffw added the enhancement New feature or request label Aug 31, 2018
@woodruffw woodruffw added the good first issue Good for newcomers label Sep 12, 2018
@technopwnsauce
Copy link

technopwnsauce commented Oct 30, 2019

Can you provide an example binary that would trigger these IMAGE_LOAD_CONFIG_DIRECTORY warning messages? What if any checks do you want to execute if this exception is encountered?

Currently you're overloading of the stream insertion operator is triggering all over your other functions - would it make more sense to have an initialization function for setting these results as fields, and have separate function(s) for managing output? This would better segregate your object construction, initialization, and output; and allow your functions to take arguments.

@woodruffw
Copy link
Member Author

Can you provide an example binary that would trigger these IMAGE_LOAD_CONFIG_DIRECTORY warning messages? What if any checks do you want to execute if this exception is encountered?

I don't have a binary on hand, sorry. An older calc.exe (from Windows XP or earlier) would probably have the truncated load config, though.

You can see the relevant checks anywhere we test loadConfigSize_, e.g.:

winchecksec/Checksec.cpp

Lines 222 to 226 in bc8e0f9

if (loadConfigSize_ < 148) {
cerr << "Warn: no or short load config, assuming no RFG"
<< "\n";
return false;
}

What I think we want is an additional method, like isTruncatedLoadConfig, which we use to populate both operator json() and operator<< for outputting. Something like:

const bool Checksec::isTruncatedLoadConfig() const {
  return loadConfigSize_ < 148;
}

Ultimately though I'm no longer sure this is a relevant piece of information to expose -- having a truncated load config is semantically equivalent to failing the relevant checks and, apart from some messages on stderr, I'm not sure if it's something we should even report on. But I'd be okay with adding it for the time being.

@woodruffw woodruffw added this to the 3.1 milestone Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest
Projects
None yet
Development

No branches or pull requests

2 participants