andymeneely / chromium-history

Scripts and data related Chromium's history
11 stars 4 forks source link

191 CVEs don't have any Rietveld IDs we scraped #104

Closed andymeneely closed 10 years ago

andymeneely commented 10 years ago

Looks like we have 191 CVEs that did have any Rietveld ids. I've put them into a new sheet in the Vulnerabilities spreadsheet. We need to look into these - could be something simple, could be more problematic. Feel free to create columns to mark examples of this.

A few potential concerns that might be culprit:

There might be other possibilities that I'm not thinking of.

bspates commented 10 years ago

At least the first two in the list are in our manual inspection sheet with reitveld ids. Is this list just those CVEs that didn't match up with those scraped in the build?

bspates commented 10 years ago

CVE-2011-2805, has issue= 7395022, which at https://codereview.chromium.org/7395022, dosen't seem to have any comments, but the link that page has to http://src.chromium.org/viewvc/chrome?view=rev&revision=91165, has two code reviews for that commit, one of which got an LGTM. I'm not really sure what the connection is between these review numbers, expect that they are linked by a commit into the trunk.

andymeneely commented 10 years ago

Yes, it's every CVE that's does not have a cvenum_code_review_id. Technically, it's the non-counting output of lib/chromium_history/verify/cve_all_verify.rb on production.

I'll look into this one.

andymeneely commented 10 years ago

So the commit you linked to is in our database, it's git hash is 97c75129f344168d6318fb8a9d4fe33de7da3485

But, the code review that it links to is different. Hm....

andymeneely commented 10 years ago

WAIT!! That commit has two code reviews. Grrrrr....

bspates commented 10 years ago

I think it actually has three, the two listed on that commit and the review where I found the link to that commit. So do we need to revise our model for a commit to have multiple reviews? Looking at the reviews, the one without an LGTM links to the one with it, so maybe we can discard the others as superfluous, especially since they have no comments.

andymeneely commented 10 years ago

Yep, we do need to change our model. We might have been missing code reviews all along so we may need to scrape more as well. Brian - can you write up the issue? I might get to it this weekend.

andymeneely commented 10 years ago

Once we fix what's going on with #105 this might be fixed. Hopefully.

andymeneely commented 10 years ago

As of d3913189ca96f85a08ba4c63ed067ec204e62090 we've fixed #105, so maybe this number will go down? I'll report back on this after tonight's build.

andymeneely commented 10 years ago

Build last night didn't really help. We're still at 192 cves that had no code reviews.

bspates commented 10 years ago

If this is still a problem we should start going over the list manually. Inspect for back ports, check if we inspected them wrong the first time (I noticed a bunch a the top of the list were from our manual inspection list.)

andymeneely commented 10 years ago

I added a new Google Spreadsheet called "Reviewless CVEs" for us to review manually. We'll use this as a big checklist to figure out what's going on. Be sure to actually correct it in the original sheets too.

andymeneely commented 10 years ago

This number might change now that's I've overhauled the loader and spreadsheets in #153 and dc7e586388bac70ad77ad5db81397e642e0778f2. After tonight's clean build I'll build up the TBD list for Kayla to track down.