Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 1101 similar vul endpoint #1154

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
17 changes: 17 additions & 0 deletions app/controllers/vulnerabilities_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,23 @@ def show
render_json_for_api @vulnerability
end

# GET /api/vulnerabilities/1/2
def similarVulnerabilities
params.permit(:id, :similarity)
valid_similarities = ['cwe', 'lessons', 'related', 'directory']
cve = params[:id]
similarity_action = params[:similarity].gsub(/same-/, '').gsub('-', '')
limit = params.key?(:limit) ? params[:limit].to_i : 10
offset = params.key?(:offset) ? params[:offset].to_i : 0
if valid_similarities.include? similarity_action
render_json_for_api(Vulnerability.similarVulnerabilities(cve, similarity_action, limit, offset))
else
render status: :bad_request, json: <<~EOS
Error: Invalid similarity option.
EOS
end
end

# GET /api/vulnerabilities/1/events
# This is an API-only route
def events
Expand Down
69 changes: 67 additions & 2 deletions app/models/vulnerability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ class Vulnerability < ApplicationRecord
has_many :events
belongs_to :project


def self.list_all(short_descriptions)
@vs = Vulnerability.
left_outer_joins(:vulnerability_tags).
Expand Down Expand Up @@ -47,7 +46,6 @@ def self.word_count
word_counts.map { |v| v.word_count }.sum
end


def tags_by_name
tags.order(name: :asc).
includes(:vulnerability_tags).
Expand All @@ -61,4 +59,71 @@ def tags_by_name
')
end

def self.similarVulnerabilities(cve, similarity_action, limit, offset)
if similarity_action == 'cwe'
query = <<~EOSQL
SELECT v2.cve, v2.description, v2.project_id, v2.upvotes, v2.nickname
FROM vulnerabilities v1
INNER JOIN vulnerability_tags vt1 ON v1.id = vt1.vulnerability_id
INNER JOIN tags t1 ON vt1.tag_id = t1.id
INNER JOIN vulnerability_tags vt2 ON vt1.tag_id = vt2.tag_id
INNER JOIN vulnerabilities v2 ON vt2.vulnerability_id = v2.id
WHERE v1.cve = ?
AND v2.cve != ?
AND starts_with(t1.shortname, 'cwe')
ORDER BY v2.upvotes DESC
LIMIT ?
OFFSET ?
EOSQL
sanitizedQuery = ActiveRecord::Base.sanitize_sql([query, cve, cve, limit, offset])
results = ActiveRecord::Base.connection.execute(sanitizedQuery);
elsif similarity_action == 'lessons'
query = <<~EOSQL
SELECT v2.cve, v2.description, v2.project_id, v2.upvotes, v2.nickname
FROM vulnerabilities v1
INNER JOIN vulnerability_tags vt1 ON v1.id = vt1.vulnerability_id
INNER JOIN tags t1 ON vt1.tag_id = t1.id
INNER JOIN vulnerability_tags vt2 ON vt1.tag_id = vt2.tag_id
INNER JOIN vulnerabilities v2 ON vt2.vulnerability_id = v2.id
WHERE v1.cve = ?
AND v2.cve != ?
AND starts_with(t1.shortname, 'Lesson')
ORDER BY v2.upvotes DESC
LIMIT ?
OFFSET ?
EOSQL
sanitizedQuery = ActiveRecord::Base.sanitize_sql([query, cve, cve, limit, offset])
results = ActiveRecord::Base.connection.execute(sanitizedQuery);
elsif similarity_action == 'directory'
query = <<~EOSQL
SELECT DISTINCT
v2.cve,
v2.description,
v2.project_id,
v2.upvotes,
v2.nickname,
v2.id as related_vul_id
FROM vulnerabilities AS v1
INNER JOIN vulnerabilities AS v2 ON (v1.id <> v2.id AND v1.project_id = v2.project_id)
INNER JOIN fixes AS f1 ON (f1.vulnerability_id = v1.id)
INNER JOIN fixes AS f2 ON (f2.vulnerability_id = v2.id)
INNER JOIN commits AS c1 ON c1.id = f1.commit_id
INNER JOIN commits AS c2 ON c2.id = f2.commit_id
INNER JOIN commit_filepaths AS cf1 ON cf1.commit_id = c1.id
INNER JOIN commit_filepaths AS cf2 ON cf2.commit_id = c2.id
INNER JOIN filepaths AS fp1 ON cf1.filepath_id = fp1.id
INNER JOIN filepaths AS fp2 ON cf2.filepath_id = fp2.id
WHERE fp1.dir = fp2.dir
AND fp1.is_code = TRUE
AND fp2.is_code = TRUE
AND fp1.project_id = fp2.project_id
AND v1.cve = ?
ORDER BY v2.upvotes DESC
LIMIT ?
OFFSET ?
EOSQL
sanitizedQuery = ActiveRecord::Base.sanitize_sql([query, cve, limit, offset])
results = ActiveRecord::Base.connection.execute(sanitizedQuery);
end
end
end
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

get '/vulnerabilities', controller: :vulnerabilities, action: :index
get '/vulnerabilities/:id', controller: :vulnerabilities, action: :show, constraints: { id: /CVE\-\d+\-\d+/ }
get '/api/vulnerabilities/:id/:similarity', controller: :vulnerabilities, action: :similarVulnerabilities, constraints: { id: /CVE\-\d+\-\d+/, similarity: /[-A-Za-z]+/ }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's actually split this out into four endpoints - :id/samedirectory, :id/samecwe, etc.. That saves us from having a giant if-statement with different queries.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would also mean the model would be different methods too, e.g. same_cwe and same_directory

get '/:id', controller: :vulnerabilities, action: :show, constraints: { id: /CVE\-\d+\-\d+/ }
get "/api/:id", controller: :vulnerabilities, action: :show, constraints: { id: /CVE\-\d+\-\d+/ }
get '/api/vulnerabilities', controller: :vulnerabilities, action: :index
Expand Down
149 changes: 98 additions & 51 deletions public/openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,83 @@ paths:
$ref: '#/components/responses/400Malformed'
"500":
$ref: '#/components/responses/500Internal'

/api/vulnerabilities/{cve}/{similarity}:
get:
summary: /api/vulnerabilities/{cve}/{similarity}
operationId: similarVulnerabilities
tags:
- vulnerabilities
description: |
Returns vulnerabilities with
parameters:
- name: cve
description: |
The CVE ID of the vulnerability to find vulnerabilities that have the similarity metric in common.
in: path
required: true
schema:
type: string
- name: similarity
description: |
The similarity metric to use.
in: path
required: true
schema:
type: string
enum:
- cwe
- Lessons
- name: limit
description: |
Limit the number of results
in: query
required: false
schema:
default: 10
type: integer
format: int32
- name: offset
description: |
Return a subset of the results, offset by this number
in: query
required: false
schema:
default: 0
type: integer
format: int32
responses:
"200":
description: OK
content:
application/json:
schema:
type: array
items:
type: object
properties:
cve:
type: string
description:
type: string
project_id:
type: integer
format: int32
upvotes:
type: integer
format: int32
nickname:
type: string
example:
- cve: CVE-2013-2878
- description: Google Chrome ~before 28.0.1500.71 allows remote attackers to cause a denial of service (out-of-bounds read) via vectors related to the handling of text.
- project_id: 980190962
- upvotes: 4
- nickname: ''
"400":
$ref: '#/components/responses/400Malformed'
"500":
$ref: '#/components/responses/500Internal'

/api/vulnerabilities/{cve}/events:
get:
Expand Down Expand Up @@ -975,41 +1052,32 @@ paths:
content:
application/json:
schema:
nodes:
type: array
items:
type: object
properties:
name:
type: string
links:
type: array
items:
type: object
properties:
source:
type: string
target:
type: integer
value:
type: integer
format: int32
example:
- nodes:
name: "project-xx"
links:
source: "project-xx"
target: "language-d"
value: 1
trend_json: '{"nodes": [{ "name": "project-xx" },{ "name": "bounty" },{ "name": "language-d" },{ "name": "lifetime-10" }],"links": [{ "source": "project-xx", "target":"language-d", "value": 1 },{ "source": "bounty", "target": "lifetime-10", "value": 2 }]}'
type: array
items:
type: object
properties:
node:
type: object
properties:
name:
type: string
link:
type: object
properties:
source:
type: string
target:
type: string
value:
type: integer
format: int32
trend_json:
type: string
"400":
$ref: '#/components/responses/400Malformed'
"500":
$ref: '#/components/responses/500Internal'




/api/corpus:
get:
summary: /api/corpus
Expand Down Expand Up @@ -1239,27 +1307,6 @@ components:
A freeform document (i.e. JSON) of any extra information we have on this record. Sometimes we don't have an exact place in our schema to place things, so we err on the side of retaining records and record them here.

Importantly, the schema of this object is subject to change. We try to keep it consistent, but all curators have a mind of their own.

sankeyNodes:
type: array
items:
type: object
properties:
name:
type: string

sankeyLinks:
type: array
items:
type: object
properties:
source:
type: string
target:
type: string
value:
type: integer

parameters:
CVEParam:
name: cve
Expand Down
27 changes: 27 additions & 0 deletions test/controllers/vulnerabilities_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,31 @@ class VulnerabilitiesControllerTest < ActionDispatch::IntegrationTest
assert_recognizes expected, "/CVE-2011-3092"
end

test 'Checks routing for similarVulnerabilities' do
assert_routing '/api/vulnerabilities/CVE-2013-2878/same-cwe', controller: "vulnerabilities", action: "similarVulnerabilities", id: "CVE-2013-2878", similarity: "same-cwe"
assert_recognizes({controller: 'vulnerabilities', action: 'similarVulnerabilities', id: 'CVE-2013-2878', similarity: 'same-cwe'}, '/api/vulnerabilities/CVE-2013-2878/same-cwe')
end


test 'Checks invalid input to similarVulnerabilities' do
get "/api/vulnerabilities/CVE-2013-2878/invalid"
assert_response :bad_request
assert_match /Error: Invalid/, @response.body
end

test 'Checks valid input to similarVulnerabilities' do
get "/api/vulnerabilities/CVE-2013-2878/same-cwe"
assert_response :success
expected = Vulnerability.similarVulnerabilities('CVE-2013-2878', 'cwe', 10, 0).to_json
assert_equal expected, @response.body
end

test 'Checks valid input to similarVulnerabilities with lessons and checks offset and limit' do
get "/api/vulnerabilities/CVE-2013-2878/same-lessons?offset=1&limit=20"
assert_response :success
expected = Vulnerability.similarVulnerabilities('CVE-2013-2878', 'lessons', 20, 1).to_json
assert_equal expected, @response.body
end


end
8 changes: 1 addition & 7 deletions test/fixtures/vulnerability_tags.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,6 @@ cve_2013_2878_language:
importance: 2
note: CWE note?

cve_2013_2878_:
vulnerability: cve_2013_2878
tag: language
importance: 2
note: CWE note?

cve_2013_2878_refactors:
vulnerability: cve_2013_2878
tag: RefactorsMethod
Expand Down Expand Up @@ -170,4 +164,4 @@ cve_2011_3093_reverts:
vulnerability: cve_2011_3093
tag: RevertsMethod
importance: 2
note: CWE note?
note: CWE note?
10 changes: 10 additions & 0 deletions test/models/vulnerability_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,14 @@ class VulnerabilityTest < ActiveSupport::TestCase
assert_nil Vulnerability.find_by(cve: "CVE-2011-3096")
end

test 'similar vulnerabilities, same-cwe' do
vuln = Vulnerability.similarVulnerabilities('CVE-2013-2878', 'cwe', 10, 0)
vulnResults = vuln.map { |v| v['cve']}
expected_results = %w(
CVE-2016-1676
)
assert_equal expected_results, vulnResults, 'SQL statement is not correct'
end


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have tests for directory, lessons, and related too.

end