Java: Add unsafe hostname verification query and remove existing overlapping query

[intrigus-lgtm]

This PR adds an improved query for hostname verification vulnerabilities to the supported query set.
The overlapping existing code in https://github.com/github/codeql/blob/main/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql is then removed, as it was easier for me to simply add a "new" query instead of duplicating/refactoring the existing query.

Todo:

• Add change note.

[smowton]

Can you remind me of the context why this is being replicated like this? I probably meant to split or move code without changing its behaviour rather than to produce (transiently) 3 identical copies of the same query, but I can't remember the original discussion off hand.

[intrigus-lgtm]

Ignore these two imports, they are for a later commit.

[intrigus-lgtm]

This kind of scenario is rather rare, of ~100 Alerts I've only seen this pattern once.

[intrigus-lgtm]

I applied all suggestions.

[smowton]

Have requested a codeowner review

[felicitymay]

@intrigus-lgtm - Apologies, my earlier suggestion to "fix" the query help was wrong.
I should have looked at the whole file and not jut the small part that the check reported.

Much of the syntax should be used as for HTML, so the <li> tags in the <recommendation> section need to be inside either <ol> or <ul> tags. However, it's not clear whether you really want a one sentence paragraph followed by a single bullet point for lines 23-27 so you might want to think about that.

You may find Query help files — CodeQL helpful. While the earlier lines weren't rejected by the PR check, I think that you're probably missing quite a bit of syntax here.

[intrigus-lgtm]

The <li> was an artifact of an older revision.
I've removed it and I think it should work now.

Is it possible for a user to generate the qhelp file themselves?
IIRC there were some changes in that area enabling the use for normal users?

[felicitymay]

@intrigus-lgtm - thanks for the most recent changes to the query help file for "Disabled hostname verification".

From the docs point of view, the text is ready to merge once CodeQL reviewers are happy.

One last comment that I'd make is that the filename, ID and @name for the query are all different and possibly it would be clearer for other users if they were more similar. I'm not sure how much effort it would be to rename the file and update the ID so that they were inline with the query @name.

[intrigus-lgtm]

@felicitymay what would you prefer?
Renaming @id and @name to java/unsafe-hostname-verification and Unsafe hostname verification?
That would be easier as I would not have to rename the files 🙂

[aschackmull]

The test output of ql/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.qlref also needs fixing.

[aschackmull]

Renaming @id and @name to java/unsafe-hostname-verification and Unsafe hostname verification?

That sounds fine to me.

We're also at the point where this should get a change note (add a file to ql/java/change-notes with a name that includes todays date, cf. the other files in that dir). See e.g. ql/java/change-notes/2020-09-21-jhipster-gen-prng-query.md for an example of another change note that also describes the addition of a new query.

[intrigus-lgtm]

Added change note, accepted test output of ql/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.qlref.