Skip to content

C++: Finalise docs for cpp/hresult-boolean-conversion and cpp/unsafe-dacl-security-descriptor#235

Merged
felicitymay merged 5 commits into
github:masterfrom
jbj:hresult-boolean-qhelp
Oct 11, 2018
Merged

C++: Finalise docs for cpp/hresult-boolean-conversion and cpp/unsafe-dacl-security-descriptor#235
felicitymay merged 5 commits into
github:masterfrom
jbj:hresult-boolean-qhelp

Conversation

@jbj

@jbj jbj commented Sep 26, 2018

Copy link
Copy Markdown
Contributor

I merged HResultBooleanConversion.qhelp and UnsafeDaclSecurityDescriptor.qhelp in #211 and #212, and I'd like to get docteam reviews for them in this follow-up PR.

I'll open a separate PR for change notes.

Cc @raulgarciamsft.

@jbj jbj requested a review from a team September 26, 2018 13:44
This commit changes the name and description of the new
`HResultBooleanConversion` query to follow our internal guidelines.
@jbj jbj changed the title C++: Finalise QHelp for cpp/hresult-boolean-conversion C++: Finalise docs for cpp/hresult-boolean-conversion and cpp/unsafe-dacl-security-descriptor Oct 1, 2018
@jbj jbj force-pushed the hresult-boolean-qhelp branch from 169e685 to 308631e Compare October 1, 2018 11:42
@jbj

jbj commented Oct 1, 2018

Copy link
Copy Markdown
Contributor Author

I've expanded the scope of this PR to include all docs work following #211 and #212. Ping @Semmle/doc.

|-----------------------------|-----------|--------------------------------------------------------------------|
| *@name of query (Query ID)* | *Tags* |*Aim of the new query and whether it is enabled by default or not* |
| Cast between HRESULT and a Boolean type (`cpp/hresult-boolean-conversion`) | external/cwe/cwe-253 | Finds logic errors caused by mistakenly treating the Windows `HRESULT` type as a Boolean instead of testing it with the appropriate macros. Enabled by default. |
| Setting a DACL to `NULL` in a `SECURITY_DESCRIPTOR` (`cpp/unsafe-dacl-security-descriptor`) | external/cwe/cwe-732 | This query finds code that creates world-writable objects on Windows by setting their DACL to `NULL`. Enabled by default. |

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two queries should have the "security" tag also.

@jbj jbj mentioned this pull request Oct 8, 2018
@itauthor

itauthor commented Oct 9, 2018

Copy link
Copy Markdown

LGTM

@jbj

jbj commented Oct 9, 2018

Copy link
Copy Markdown
Contributor Author

I just resolved the changenote conflict in the web-based conflict editor on github.com. I hope that worked.

@jbj

jbj commented Oct 10, 2018

Copy link
Copy Markdown
Contributor Author

@itauthor if you're happy with these changes, please approve and merge.

@felicitymay felicitymay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbj - it seems that an admin hitch means that @itauthor hasn't been added to the organization yet, so merging on his behalf.

@felicitymay felicitymay merged commit e262972 into github:master Oct 11, 2018
aibaars pushed a commit that referenced this pull request Oct 14, 2021
Move comment so it's not treated as part of the precision metadata
smowton pushed a commit to smowton/codeql that referenced this pull request Feb 7, 2022
…erface

Extract companion objects from interfaces
MathiasVP added a commit to MathiasVP/ql that referenced this pull request Aug 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants