Skip to content

Commit

Permalink
various improvements
Browse files Browse the repository at this point in the history 
* cleaner yml emitter
* remove _instructions from yml
* add "important links"
* fix upvotes null issue
* various writing things in the templates
* small scss tweaks
* reindent index.html.etb
  • Loading branch information
@andymeneely
andymeneely committed Oct 25, 2023
1 parent 83f9580 commit b81c3f1
Show file tree
Hide file tree
Showing 8 changed files with 195 additions and 144 deletions.
55 changes: 41 additions & 14 deletions app/assets/javascripts/curate/curationwizard.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import "select2/dist/js/select2";
import { vhpMarkdown } from "../global/vhpMarkdown";
import Inputmask from "inputmask";
import moment from "moment/moment";
import YAML from 'yaml'

const parentFields = [];
const childFields = [];
Expand Down Expand Up @@ -286,23 +287,32 @@ function checkForSavedAnswer(fieldName) {
}
return fieldValue;
}


function populateYMLField(progress) {
const yamlField = document.getElementById("YAMLOutput");
yamlField.value = "";
//console.log(progress);
if (progress && Object.keys(progress).some((key) => key === "fieldName")) {
progress.forEach(function (savedQuestion) {
yamlField.value +=
savedQuestion.fieldName +
": " +
JSON.stringify(savedQuestion.fieldVal) +
"\r\n";
});
} else {
Object.entries(progress).forEach(([key, value]) => {
yamlField.value += `${key}: ${JSON.stringify(value)}\r\n`;
});
}
// TODO For now, remove all "_instructions" keys from the yml right here. Eventually we need a cleaner way of handling that.
progress = Object.fromEntries(Object.entries(progress).filter(([k,_v]) => !k.includes('_instructions' )))

// console.log("Populating YAML with this: ")
// console.log(progress);
yamlField.value = YAML.stringify(progress)

// FIXME Old YML emitter wasn't clean enough - BUT, I'm not sure what hte first half of the if-statement is doing here so I'm going to ask.
// if (progress && Object.keys(progress).some((key) => key === "fieldName")) {
// progress.forEach(function (savedQuestion) {
// yamlField.value +=
// savedQuestion.fieldName +
// ": " +
// JSON.stringify(savedQuestion.fieldVal) +
// "\r\n";
// });
// } else {
// Object.entries(progress).forEach(([key, value]) => {
// yamlField.value += `${key}: ${JSON.stringify(value)}\r\n`;
// });
// }
}
function openYml() {
$("#open-yaml").trigger("click");
Expand Down Expand Up @@ -841,6 +851,8 @@ function buildQuestionsAccordion(questions) {

html += `</select>`;
}

html += populateImportantLinks(currentCVE, project)
}
html += `<ul class="accordion" id="${accordionElementId}" data-accordion data-allow-all-closed="true" data-multi-expand="false">`;
parentFields.forEach((q, index) => {
Expand Down Expand Up @@ -943,6 +955,15 @@ function buildQuestionsAccordion(questions) {
bindRepeaters();
}

function populateImportantLinks(cve, project) {
const nvdUrl = `https://nvd.nist.gov/vuln/detail/${cve}`
const githubUrl = `https://github.com/VulnerabilityHistoryProject/vulnerabilities/blob/dev/cves/${project}/${cve}.yml`
return `
<a href="${nvdUrl}" class="important-links" target="blank" id="cveLink">${cve} on nvd.nist.gov</a>
<a href="${githubUrl}" class="important-links" target="blank" id="ymlGitHubLink">${cve}.yml on our GitHub</a>
`
}

function setConditionalToggle(event) {
let fieldValue = event.target.value ? event.target.value : "";
let subFieldParent = event.target.dataset.fieldname;
Expand Down Expand Up @@ -1469,6 +1490,11 @@ export default function onCurate() {
url: `/api/vulnerabilities/${cveSelected}`,
dataType: "json",
}).then((data) => {
//FIXME fixing upvotes in a janky way because I'm on a deadline
if(data.notes.upvotes === null) {
data.notes.upvotes = 0
}

let project = getProjectSubdomain(data.project_id);
let thisCVE =
getCurationInProgress(cveSelected) ??
Expand All @@ -1489,6 +1515,7 @@ export default function onCurate() {
}

let newProgress = currentProgress;

localStorage.setItem("VHPWizardStatus", JSON.stringify(newProgress));
populateYMLField(data.notes);
loadQuestions(project);
Expand Down
12 changes: 2 additions & 10 deletions app/assets/javascripts/curate/questions/kernel.json
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
[
{
"key": "reported_date",
"label": "Reported Date",
"summary": "What date was the vulnerability reported to the security team?",
"instructions": "What date was the vulnerability reported to the security team? Leave blank if no date is given.",
"howToFind": "Look at the security bulletins and bug reports. It is not necessarily the same day that the CVE was created.",
"defaultValue": "",
"type": "date"
}, {
{
"key": "bounty",
"label": "Bounties",
"summary": "Was a bounty paid out for this vulnerability? How much? When?",
Expand Down Expand Up @@ -53,7 +45,7 @@
"key": "bugs_repeater",
"label": "Bugs",
"defaultValue": 1,
"type": "repeater",
"type": "repeater",
"summary":"What bugs were involved with this vulnerability?",
"instructions": "What bugs and/or pull requests are involved in this vulnerability? <br/><br/>For systemd, this is typically their GitHub issues, but could also include bugs from other databases. Put a URL instead of a single number.",
"entryLimit": 3
Expand Down
64 changes: 36 additions & 28 deletions app/assets/javascripts/curate/questions/shared.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,19 @@
{
"key": "CVE",
"label": "CVE Identifier",
"summary": "What CNA-assigned number has been used to refer to this vulnerability?",
"summary": "What CVE number has been used to refer to this vulnerability?",
"instructions": "This is the official identifier assigned by the Common Vulnerabilities and Exposures standard. ",
"defaultValue": "",
"type": "cve-selector",
"validationType": "cve",
"required": true
},
{
"key": "CWE",
"key": "CWE Identifier",
"label": "CWE",
"instructions": "Please go to <a href='https://cwe.mitre.org' target='_blank'>cwe.mitre.org</a> and find the most specific, appropriate CWE entry that describes your vulnerability.<br/><br/>If you have anything to note about why you classified it this way, write something in CWE_note. This field is optional.<br/><br/>Just the number here is fine. No need for name or CWE prefix. If more than one apply here, then choose the best one and mention the others in CWE_note.",
"howToFind": "We recommend going to <a href='https://cwe.mitre.org/data/definitions/699.html' target='_blank'>https://cwe.mitre.org/data/definitions/699.html</a> for the Software Development view of the vulnerabilities. We also recommend the tool <a href='http://www.cwevis.org/viz' target='_blank'>http://www.cwevis.org/viz</a> to help see how the classifications work.",
"summary": "What type of vulnerability was it?",
"instructions": "Please go to <a href='https://cwe.mitre.org' target='_blank'>cwe.mitre.org</a> and find the most specific, appropriate CWE entry that describes your vulnerability. If you have anything to note about why you classified it this way, write something in CWE_note. Do <i>not</i> choose a broad category or view, such as CWE-699 or CWE-1006. ",
"howToFind": "We recommend going to <a href='https://cwe.mitre.org/data/definitions/699.html' target='_blank'>the Software Development view</a> for of the vulnerabilities. You can also read <a href='https://cwe.mitre.org/documents/cwe_usage/guidance.html'>their guidance</a> for mapping from CVE to the CWE. ",
"defaultValue": "",
"type": "cwe-selector",
"validationType": "number"
Expand All @@ -28,31 +30,34 @@
},{
"key": "nickname",
"label": "Nickname",
"instructions": "A catchy name for this vulnerability that would draw attention it. If the report mentions a nickname, use that.",
"instructions": "A catchy name for this vulnerability that would draw attention it. If the report mentions a nickname, use that. Must be under 30 characters.",
"defaultValue": "",
"type": "input"
},{
"key": "announced",
"label": "Announced Date",
"summary": "Was there a date that this vulnerability was announced to the world?",
"summary": "When was it announced?",
"howToFind": "You can find this in changelogs, blogs, bug reports, or perhaps the CVE date.",
"defaultValue": "",
"type": "date"
}, {
"key": "description",
"label": "Description",
"summary": "In your own words, write a description. Make it easy for anyone with experience to read.",
"instructions": "You can get an initial description from the CVE entry on cve.mitre.org. These descriptions are a fine start, but they can be kind of jargony.<br/><br/>Rewrite this description IN YOUR OWN WORDS. Make it interesting and easy to read to anyone with some programming experience. We can always pull up the NVD description later to get more technical.<br/><br/>Try to still be specific in your description, but remove project-specific stuff. Remove references to versions, specific filenames, and other jargon that outsiders to this project would not understand. Technology like 'regular expressions' is fine, and security phrases like 'invalid write' are fine to keep too.",
"summary": "In your own words, write a description.",
"instructions": "You can get an initial description from the CVE entry on cve.mitre.org. However, while these descriptions are a fine start, they can be kind of jargony. Rewrite this description IN YOUR OWN WORDS. Make it interesting and easy to read to anyone with some programming experience. We can always pull up the NVD description later to get more technical. Try to still be specific in your description, but remove project-specific stuff. Remove references to versions, specific filenames, and other jargon that outsiders to this project would not understand. Technology like 'regular expressions' is fine, and security phrases like 'invalid write' are fine to keep too.",
"defaultValue": "",
"type": "textarea"
},{
},
{
"key": "fixes",
"label": "Fixes",
"summary": "Are there published fix or patch dates",
"instructions": "Please put the SVN commit number in 'commit,' and any notes about how this was discovered in the 'note' field.",
"label": "Fix Commits",
"summary": "What are the fix commits?",
"instructions": "This must be a git commit hash from the source repo, a 40-character hexademical string.",
"defaultValue": 1,
"type": "repeater"
}, {
},

{
"key": "fixes_commit",
"label": "Commit",
"defaultValue": "",
Expand All @@ -68,12 +73,14 @@
"parentField" : "fixes",
"acceptedValues" : 1,
"comparisonType" : "IN"
}, {
},

{
"key": "vccs",
"label": "VCCS",
"summary": "Please enter the commit hashes that contributed to this vulnerability",
"instructions": "The vulnerability-contributing commits. Look up these VCC commits and verify that they are not simple refactorings, and that they are, in fact introducing the vulnerability into the system. Often, introducing the file or function is where the VCC is, but VCCs can be anything.",
"howToFind": "These are found by our tools by traversing the Git Blame history, where we determine which commit(s) introduced the functionality.<br/><br/>",
"label": "Origin Commits (VCC)",
"summary": "What are the Vulnerability-Contributing Commits?",
"instructions": "VCCs are the vulnerability-contributing commits. Look up these VCC commits and verify that they are not simple refactorings, and that they are, in fact introducing the vulnerability into the system. Often, introducing the file or function is where the VCC is, but VCCs can be anything.",
"howToFind": "These are found by our tools by traversing the Git Blame history, where we determine which commit(s) introduced the functionality. ",
"defaultValue": 1,
"type": "repeater",
"entryLimit": 3
Expand All @@ -96,14 +103,15 @@
}, {
"key": "upvotes",
"label": "Upvotes",
"instructions": "For the first round, ignore this upvotes number.<br/><br/>For the second round of reviewing, you will be giving a certain amount of upvotes to each vulnerability you see. Your peers will tell you how interesting they think this vulnerability is, and you'll add that to the upvotes score on your branch.",
"defaultValue": "",
"summary": "How interesting is this?",
"instructions": "Upvotes are an entirely subjective measure of how interesting you think this vulnerability is, for whatever reason. If the score is currently zero (or null), rate this vulnerability on a scale of 1 to 10. Otherwise, add a score of 1 to 5 to the existing score.",
"defaultValue": 0,
"type": "input",
"validationType": "number"
},{
"key": "discovered",
"label": "Discovered",
"instructions": "How was this vulnerability discovered?<br/><br/>Answer in longform below in 'answer.'<br/><br/>If there is no evidence as to how this vulnerability was found, then please explain where you looked.",
"instructions": "How was this vulnerability discovered? Answer in longform below in 'answer.' If there is no evidence as to how this vulnerability was found, then please explain where you looked.",
"howToFind": "Go to the bug report and read the conversation to find out how this was originally found. ",
"defaultValue": "",
"type": "input"
Expand Down Expand Up @@ -168,7 +176,7 @@
"instructions": "Is it plausible that a fully automated tool could have discovered this? These are tools that require little knowledge of the domain, e.g. automatic static analysis, compiler warnings, fuzzers.",
"defaultValue": 1,
"type": "content",
"examples": "For true answers: <ul><li>SQL injection</li><li>XSS</li><li>buffer overflow</li><li>use-after-free</li></ul><br/><br/>For false answers: <ul><li>specification violations</li><li>permission issues</li><li>anything that would require a tool to be 'aware' of the project's domain-specific requirements</li></ul>"
"examples": "For true answers: <ul><li>SQL injection</li><li>XSS</li><li>buffer overflow</li><li>use-after-free</li></ul> For false answers: <ul><li>specification violations</li><li>permission issues</li><li>anything that would require a tool to be 'aware' of the project's domain-specific requirements</li></ul>"
}, {
"key": "autodiscoverable_answer",
"label": "",
Expand Down Expand Up @@ -552,7 +560,7 @@
}, {
"key": "mistakes",
"label": "Mistakes",
"instructions": "In your opinion, after all of this research, what mistakes were made that led to this vulnerability? Coding mistakes? Design mistakes? Maintainability? Requirements? Miscommunications?<br/><br/>There can, and usually are, many mistakes behind a vulnerability.<br/><br/>These are grey areas, of course. But do your best to analyze the mistakes according to this framework.<br/><br/>Look at the CWE entry for this vulnerability and examine the mitigations they have written there. Are they doing those? Does the fix look proper?<br/><br/>Write a thoughtful entry here that those in the software engineering industry would find interesting.",
"instructions": "In your opinion, after all of this research, what mistakes were made that led to this vulnerability? Coding mistakes? Design mistakes? Maintainability? Requirements? Miscommunications? There can, and usually are, many mistakes behind a vulnerability. These are grey areas, of course. But do your best to analyze the mistakes according to this framework. Look at the CWE entry for this vulnerability and examine the mitigations they have written there. Are they doing those? Does the fix look proper? Write a thoughtful entry here that those in the software engineering industry would find interesting.",
"defaultValue": "",
"type": "textarea",
"examples": "Remember that mistakes can come in many forms:<ul><li>slip: failing to complete a properly planned step due to inattention e.g. wrong key in the ignition e.g. using < instead of <= </li><li>lapse: failing to complete a properly planned step due to memory failure e.g. forgetting to put car in reverse before backing up e.g. forgetting to check null</li><li>planning error: error that occurs when the plan is inadequate e.g. getting stuck in traffic because you didn't consider the impact of the bridge closing e.g. calling the wrong method e.g. using a poor design</li></ul>"
Expand Down Expand Up @@ -591,7 +599,7 @@
}, {
"key": "discussion",
"label": "Discussion",
"instructions": "Was there any discussion surrounding this?<br/><br/>A discussion can include debates, disputes, or polite talk about how to resolve uncertainty.<br/<br/>Just because you see multiple comments doesn't mean it's a discussion. For example:<ul><li>'Fix line 10.' 'Ok' is not what we call a discussion</li><li>'Ping' (reminding people)</li></ul>",
"instructions": "Was there any discussion surrounding this? A discussion can include debates, disputes, or polite talk about how to resolve uncertainty.<br/<br/>Just because you see multiple comments doesn't mean it's a discussion. For example:<ul><li>'Fix line 10.' 'Ok' is not what we call a discussion</li><li>'Ping' (reminding people)</li></ul>",
"howToFind": "Check the bugs reports, pull requests, and mailing lists archives.",
"defaultValue": 1,
"type": "content",
Expand Down Expand Up @@ -638,7 +646,7 @@
}, {
"key": "vouch",
"label": "Vouch",
"instructions": "Was there any part of the fix that involved one person vouching for another's work?<br/><br/>This can include:<ul><li>signing off on a commit message</li><li>mentioning a discussion with a colleague checking the work</li><li>upvoting a solution on a pull request</li></ul>",
"instructions": "Was there any part of the fix that involved one person vouching for another's work? This can include:<ul><li>signing off on a commit message</li><li>mentioning a discussion with a colleague checking the work</li><li>upvoting a solution on a pull request</li></ul>",
"defaultValue": 1,
"type": "content"
}, {
Expand Down Expand Up @@ -668,7 +676,7 @@
}, {
"key": "stacktrace",
"label": "Stacktrace",
"instructions": "Are there any stacktraces in the bug reports?<br/><br/>Secondly, if there is a stacktrace, is the fix in the same file that the stacktrace points to?<br/><br/>If there are no stacktraces, then both of these are false - but be sure to mention where you checked in the note.",
"instructions": "Are there any stacktraces in the bug reports? Secondly, if there is a stacktrace, is the fix in the same file that the stacktrace points to? If there are no stacktraces, then both of these are false - but be sure to mention where you checked in the note.",
"defaultValue": 1,
"type": "content"
}, {
Expand Down Expand Up @@ -713,7 +721,7 @@
}, {
"key": "forgotten_check",
"label": "Forgotten Check",
"instructions": "Does the fix for the vulnerability involve adding a forgotten check?<br/><br/>A 'forgotten check' can mean many things. It often manifests as the fix inserting an entire if-statement or a conditional to an existing if-statement. Or a call to a method that checks something.",
"instructions": "Does the fix for the vulnerability involve adding a forgotten check? A 'forgotten check' can mean many things. It often manifests as the fix inserting an entire if-statement or a conditional to an existing if-statement. Or a call to a method that checks something.",
"defaultValue": 1,
"type": "content",
"examples": ["null pointer checks","check the current role, e.g. root","boundary checks for a number","consult file permissions","check a return value"]
Expand Down Expand Up @@ -744,7 +752,7 @@
}, {
"key": "order_of_operations",
"label": "Order of Operations",
"instructions": "Does the fix for the vulnerability involve correcting an order of operations?<br/><br/>This means the fix involves moving code around or changing the order of how things are done.",
"instructions": "Does the fix for the vulnerability involve correcting an order of operations? This means the fix involves moving code around or changing the order of how things are done.",
"defaultValue": 1,
"type": "content"
}, {
Expand Down
Loading

0 comments on commit b81c3f1

Please sign in to comment.