Issue #82 Adds -d and -V flags, Add unsupported RFG notice#83
Issue #82 Adds -d and -V flags, Add unsupported RFG notice#83bonafideduck wants to merge 2 commits intotrailofbits:masterfrom
Conversation
|
|
|
Thanks @bonafideduck! I'll take a look at this in a bit. |
|
It seems to be failing in the tests. I did my work on linux, so perhaps there is a difference in the compilers. My c++ is not strong, so perhaps it will be obvious to you. |
| constexpr const char kRFGDescription[] = | ||
| "Binaries with RFG enabled have additional return-oriented-programming " | ||
| "protections."; | ||
| // https://www.techrepublic.com/article/windows-10-security-how-the-shadow-stack-will-help-to-keep-the-hackers-at-bay/ |
There was a problem hiding this comment.
This link is pretty light on details, consider replacing it with this one: https://xlab.tencent.com/en/2016/11/02/return-flow-guard/
Also, please put it above the variable declaration 🙂
| "Binaries with RFG enabled have additional return-oriented-programming " | ||
| "protections."; | ||
| // https://www.techrepublic.com/article/windows-10-security-how-the-shadow-stack-will-help-to-keep-the-hackers-at-bay/ | ||
| "(This was never released by Microsoft). Binaries with RFG enabled have " |
There was a problem hiding this comment.
NIt: Remove the parens and put this sentence at the end of the description.
| }; | ||
| } | ||
|
|
||
| const std::string Checksec::detailedReport() const { |
There was a problem hiding this comment.
I don't think we currently want this functionality: it'll be a little unwieldy to maintain (since we don't currently have a nice way to just enumerate all of our checks), and its existence on the main Checksec class is another place where we fail to isolate with CLI from the C++ public API.
I'm happy to merge the rest of this PR, though.
|
|
||
| const std::string Checksec::detailedReport() const { | ||
| json j = toJson(); | ||
| std::string s = ""; |
There was a problem hiding this comment.
Since you mentioned not being experienced with C++: the idiomatic way to do this kind of function is with a std::stringstream, which you can then feed chunks with the stream-style << operator. You can then pull the ultimate std::string out of the stream with std::stringstream::str().
|
Thanks for submitting this. I think we're going to pass on the |
|
It is your tool. So I wont push hard. I wish you would reconsider. Although visible in the json, statements like something not being possible on 64 bits in an easily readable format saves a lot of time. |
Yeah, I'm sympathetic. I do think this is a feature we'd like in the medium-term, but the current Checksec API is misdesigned in a few places that make me wary of adding it as-is. To sum them up:
|
|
Hi, I apologize for not updating this PR. I was waiting for a response from Zoom Legal about the Contributor License Agreement. They said that this was a bit too broad of an agreement in that I wasn't just signing for this action, but the actions of anyone in Zoom. Legal said they'd be willing to discuss a less broad license. |
|
@bonafideduck No problem! I'll raise this internally today and see if we can come up with a solution. |
|
@bonafideduck I think there may be some confusion here. This CLA should only apply to you, individually, not your entire company. Can you have them email me @ dan@trailofbits.com and I can sort it out? Thanks! |
#82