Add Email Content Injection Query #108
Conversation
| @@ -0,0 +1,16 @@ | |||
| /** | |||
| * @name Email content injection | |||
| * @description This query tests for Email content injection ie. instances where a | |||
There was a problem hiding this comment.
ie. should probably be i.e. (id est = that is)
There was a problem hiding this comment.
We generally prefer not to use abbreviations like "i.e." or "e.g." at all. It is almost always clearer to use a spelled-out alternative like "that is" or "for example" instead.
ghost
marked this pull request as ready for review
|
I have added tests and necessary documentation. This can now be reviewed and merged. |
max-schaefer
left a comment
max-schaefer
left a comment
There was a problem hiding this comment.
Thank you very much for your contribution! Here is an initial batch of comments, I'm planning to take a more thorough look at a later time.
Have you run this query on any projects on LGTM.com yet?
max-schaefer
left a comment
max-schaefer
left a comment
There was a problem hiding this comment.
Here are a few more detailed comments, overall this looks very nice!
We'll evaluate it on LGTM.com and @sauyon can help you with auto-stubbing the libraries the tests depend on.
ql/test/library-tests/semmle/go/frameworks/Email/MailDataFunction.ql
Outdated
Show resolved
Hide resolved
| * Extend this class to refine existing API models. If you want to model new APIs, | ||
| * extend `MailData::Range` instead. | ||
| */ | ||
| class MailData extends DataFlow::Node { |
There was a problem hiding this comment.
I have removed the Function suffix and renamed this as MailData.
|
The evaluation came back reasonable. I'll leave it up to you whether you want to address the false negative we discussed on Slack. Other than that, I think the only thing that's missing is replacing the manually created dependency stubs with auto-generated ones, which @sauyon can take care of. |
|
Hm, sorry about the comment spam and then deletion; GitHub helpfully had comments from before staged and didn't show them to me in the review screen because they were outdated. |
This adds a query for Email content injection issues. It models the Golang's net/smtp library as well as the Sendgrid email library (581 stars).
|
I have updated the module name as well as the corresponding qldoc. I have squashed all the commits into one and rebased the branch on origin/master for final merge. |
|
@max-schaefer , I am trying to model the |
|
I've taken this opportunity to push the stub commit. |
max-schaefer
added
the
needs-polishing
max-schaefer
removed
the
needs-polishing
This adds a query for Email content injection issues. It models the Golang's net/smtp library as well as the Sendgrid email library (581 stars).
3 participants
This adds a query for Email content injection issues.
It models the Golang's net/smtp library as well as the Sendgrid library (581 stars)
I am still working on the tests and documentation. I will add those soon. But I would still like a review until then.