Skip to content
This repository was archived by the owner on Jan 5, 2023. It is now read-only.

CWE-322 InsecureHostKeyCallback #234

Merged
merged 20 commits into from
Jun 30, 2020
Merged

CWE-322 InsecureHostKeyCallback #234

merged 20 commits into from
Jun 30, 2020

Conversation

rvermeulen
Copy link
Contributor

Add support for the detection of insecure host key callback implementations that accept any host key when establishing a SSH connection.

@rvermeulen rvermeulen changed the title CWE-322 InsecureHostKeyCallback query CWE-322 InsecureHostKeyCallback Jun 29, 2020
@max-schaefer max-schaefer self-assigned this Jun 30, 2020
max-schaefer
max-schaefer suggested changes Jun 30, 2020
View reviewed changes
Copy link
Contributor

@max-schaefer max-schaefer left a comment

Choose a reason for hiding this comment

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

Thank you very much for your contribution, and well done for using depstubber!

I have a few comments, but seeing as this is an experimental contribution they are optional, we can just merge this and clean it up later if you prefer.

rvermeulen and others added 13 commits June 30, 2020 11:13
@rvermeulen @max-schaefer
Update ql/src/experimental/CWE-322/InsecureHostKeyCallback.qhelp
31e4e4e 
Co-authored-by: Max Schaefer <54907921+max-schaefer@users.noreply.github.com>
@rvermeulen @max-schaefer
Update ql/src/experimental/CWE-322/InsecureHostKeyCallback.qhelp
9c2d284 
Co-authored-by: Max Schaefer <54907921+max-schaefer@users.noreply.github.com>
@rvermeulen @max-schaefer
Update ql/src/experimental/CWE-322/InsecureHostKeyCallback.qhelp
35ac92e 
Co-authored-by: Max Schaefer <54907921+max-schaefer@users.noreply.github.com>
@rvermeulen @max-schaefer
Update ql/src/experimental/CWE-322/InsecureHostKeyCallback.ql
4cb109d 
Co-authored-by: Max Schaefer <54907921+max-schaefer@users.noreply.github.com>
@rvermeulen @max-schaefer
Update ql/src/experimental/CWE-322/InsecureHostKeyCallback.ql
7eddb83 
Co-authored-by: Max Schaefer <54907921+max-schaefer@users.noreply.github.com>
@rvermeulen
Remove CWE reference
515efcd 
This is automatically added
@rvermeulen
Rephrase overview
df5ab79
@rvermeulen
Update examples in help file
ee4747a 
Show both insecure examples and a secure example.
@rvermeulen
Shorten query id
044e103
@rvermeulen
Add getAResult member predicate to FunctionNode
4844683
@rvermeulen
Switch to standard member predicate getAResult
2600405 
The DataFlow::FunctionNode class provides the member predicate getAResult that gets a value returned by the underlying function, if any.
@rvermeulen
Auto-format query
f46bb11
@rvermeulen
Reword overview to use more specific wording
783b65a 
Changing hijack attack, describing multiple different possible attacks,
to Machine-in-the-Middle attack that describe the exact attack
that can be performed when an insecure host key callback is used.
max-schaefer
max-schaefer suggested changes Jun 30, 2020
View reviewed changes
Copy link
Contributor

@max-schaefer max-schaefer left a comment

Choose a reason for hiding this comment

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

A final round of minor nitpicks, then we should be good to go.

rvermeulen and others added 4 commits June 30, 2020 15:35
@rvermeulen @max-schaefer
Update wording in getAResult comment
194d585 
ResultNode covers both cases. Regarding "if any", we normally only use that for predicates that have at most one result.

Co-authored-by: Max Schaefer <54907921+max-schaefer@users.noreply.github.com>
@rvermeulen @max-schaefer
Correct typo in help file
c36ae00 
Co-authored-by: Max Schaefer <54907921+max-schaefer@users.noreply.github.com>
@rvermeulen
Inline class HostKeyCallbackField
dd3690d
@rvermeulen
Add golang/crypto license to generated stub
0253e37 
The generated stub copies code from golang/crypto.
@rvermeulen
Copy link
Contributor Author

@max-schaefer since this PR updates FunctionNode, how would we update its documentation?

@max-schaefer
Copy link
Contributor

@max-schaefer since this PR updates FunctionNode, how would we update its documentation?

That will get updated automatically at some point. No need to do anything about it in this PR.

@rvermeulen
Copy link
Contributor Author

rvermeulen commented Jun 30, 2020 •
edited
Loading

Thanks @max-schaefer, @adityasharad, @brompwnie, and @intrigus-lgtm for contributing ✨

@max-schaefer
Copy link
Contributor

Many thanks @rvermeulen for your contribution!

@max-schaefer max-schaefer merged commit a89b87f into github:master Jun 30, 2020
@max-schaefer max-schaefer added the needs-polishing An external contribution that may need follow-up work. label Jun 30, 2020
@intrigus-lgtm
Copy link
Contributor

intrigus-lgtm commented Jun 30, 2020 •
edited
Loading

@rvermeulen to give you the same tip Max gave me on my first PR:

^ Pro tip: in the "Files changed" view, you have the option of adding review suggestions to a batch and commit them all at once.

(Although this PR got squashed it seems, so in this case it didn't matter)

@rvermeulen
Copy link
Contributor Author

rvermeulen commented Jun 30, 2020 •
edited
Loading

@rvermeulen to give you the same tip Max gave me on my first PR:

^ Pro tip: in the "Files changed" view, you have the option of adding review suggestions to a batch and commit them all at once.

(Although this PR got squashed it seems, so in this case it didn't matter)

Thanks! That is a great suggestion. It felt off not being able to batch the suggested code changes, but now I known it is possible.

ceh pushed a commit to ceh-forks/codeql-go that referenced this pull request Jul 22, 2020
Merge pull request github#234 from sauyon/reflectedxss-fixes
61ee9a4 
ReflectedXss: Remove FPs from constant prefix Fprintfs
@max-schaefer max-schaefer mentioned this pull request Jul 30, 2020
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@brompwnie brompwnie brompwnie left review comments

@intrigus-lgtm intrigus-lgtm intrigus-lgtm left review comments

@max-schaefer max-schaefer max-schaefer approved these changes

Assignees

@max-schaefer max-schaefer

Labels
needs-polishing An external contribution that may need follow-up work.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rvermeulen @max-schaefer @intrigus-lgtm @brompwnie