Skip to content

Conversation

@jbj
Copy link
Contributor

@jbj jbj commented Sep 26, 2018

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
Copy link
Contributor Author

jbj commented Oct 1, 2018

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
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
Copy link

itauthor commented Oct 9, 2018

LGTM

@jbj
Copy link
Contributor Author

jbj commented Oct 9, 2018

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

@jbj
Copy link
Contributor Author

jbj commented Oct 10, 2018

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

Copy link
Contributor

@felicitymay felicitymay left a comment

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