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

Conversation

gagliardetto
Copy link
Contributor

@gagliardetto gagliardetto commented Sep 21, 2020

I excluded functions/methods that encrypt or sign data.


codebox commands:

codebox --out-dir=./generated/latest --pkg=/usr/local/go/src/crypto --http
codebox --out-dir=./generated/latest --pkg=/usr/local/go/src/crypto/cipher --http
#codebox --out-dir=./generated/latest --pkg=/usr/local/go/src/crypto/ecdsa --http
#codebox --out-dir=./generated/latest --pkg=/usr/local/go/src/crypto/ed25519 --http
codebox --out-dir=./generated/latest --pkg=/usr/local/go/src/crypto/rsa --http
codebox --out-dir=./generated/latest --pkg=/usr/local/go/src/crypto/tls --http
codebox --out-dir=./generated/latest --pkg=/usr/local/go/src/crypto/x509 --http

Part of #167

@gagliardetto
Copy link
Contributor Author

After all the remaining PRs are merged, would it be possible to run a final LGTM compare (or whatever that is) between the before-stdlib, and after-stdlib?

@smowton
Copy link
Contributor

smowton commented Sep 22, 2020

That's actually quite difficult, as lgtm.com won't let you type that much text into the query console! I've had to do it in batches so far to work around that limit, so will have missed some cases that only occur with several models in place at the same time. I can do that for a much smaller project set that we use for performance measurement, though.

@gagliardetto
Copy link
Contributor Author

gagliardetto commented Sep 22, 2020

That's actually quite difficult, as lgtm.com won't let you type that much text into the query console! I've had to do it in batches so far to work around that limit, so will have missed some cases that only occur with several models in place at the same time. I can do that for a much smaller project set that we use for performance measurement, though.

Understandable.

So, in lgtm.com, there's no way to choose which version is used to do the scanning? (internally)

lgtm.com currently runs the latest https://github.com/github/codeql-go/tree/lgtm.com, right?

That means that if a scan is done now, and then a scan after all is merged and promoted to the lgtm.com branch, a compare would be possible? (which could take a few months, right?)


Why I'm asking:

  1. To evaluate speed changes.
  2. To find new bugs, and have a better understanding of how and where the added tait-flow models helped.
  3. To make a better case for a bounty enquiry.

Should I make another PR that assembles all these PRs?

@smowton
Copy link
Contributor

smowton commented Sep 23, 2020

Speed changes are likely accounted for -- I've run several whole-distribution, all-queries tests (with a restricted sampling of projects) with no worrisome performance findings.

@smowton smowton merged commit a714863 into github:main Sep 23, 2020
@smowton
Copy link
Contributor

smowton commented Sep 23, 2020

Regarding what helped:

All using the reflected-XSS query to evaluate,
#317 (comment) reports 8 new probable true positives with causes
#337 (comment) reports 5 new probable true positives, I think all from more thorough coverage of net/http functions.
#342 (comment) reports probable 10-15 new true positives, almost all due to fmt.Errorf modelling

So in terms of total impact, I'd be wiling to testify to 25 new probable true positives with just that one query, and doubtless others elsewhere. I'm afraid I don't have the time to do the totally ideal evaluation (all queries x all projects + hand classification of all the new results).

@gagliardetto
Copy link
Contributor Author

Regarding what helped:

All using the reflected-XSS query to evaluate,
#317 (comment) reports 8 new probable true positives with causes
#337 (comment) reports 5 new probable true positives, I think all from more thorough coverage of net/http functions.
#342 (comment) reports probable 10-15 new true positives, almost all due to fmt.Errorf modelling

Thanks @smowton for gathering them in one place!

So in terms of total impact, I'd be wiling to testify to 25 new probable true positives with just that one query, and doubtless others elsewhere.

That would be awesome! Thanks 😄

I'm afraid I don't have the time to do the totally ideal evaluation (all queries x all projects + hand classification of all the new results).

Yeah, that would be a titanic task that would take quite a lot of time. I'll postpone it for the time being (maybe indefinitely).


Thanks for everything @smowton ! have a great evening! 🎆 🎆 🎆

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants