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

Add apache_httpd improver #1102

Merged
merged 2 commits into from
Jan 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 97 additions & 2 deletions vulnerabilities/importers/apache_httpd.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,17 @@
# See https://aboutcode.org for more information about nexB OSS projects.
#

import asyncio
import logging
import urllib
from datetime import datetime
from typing import Iterable
from typing import List
from typing import Mapping
from typing import Optional

import requests
from bs4 import BeautifulSoup
from django.db.models.query import QuerySet
from packageurl import PackageURL
from univers.version_constraint import VersionConstraint
from univers.version_range import ApacheVersionRange
Expand All @@ -21,8 +27,20 @@
from vulnerabilities.importer import AffectedPackage
from vulnerabilities.importer import Importer
from vulnerabilities.importer import Reference
from vulnerabilities.importer import UnMergeablePackageError
from vulnerabilities.importer import VulnerabilitySeverity
from vulnerabilities.improver import Improver
from vulnerabilities.improver import Inference
from vulnerabilities.models import Advisory
from vulnerabilities.package_managers import GitHubTagsAPI
from vulnerabilities.package_managers import VersionAPI
from vulnerabilities.severity_systems import APACHE_HTTPD
from vulnerabilities.utils import AffectedPackage as LegacyAffectedPackage
from vulnerabilities.utils import get_affected_packages_by_patched_package
from vulnerabilities.utils import nearest_patched_package
from vulnerabilities.utils import resolve_version_range

logger = logging.getLogger(__name__)


class ApacheHTTPDImporter(Importer):
Expand Down Expand Up @@ -147,7 +165,7 @@ def fetch_links(url):
return links
Copy link
Member

Choose a reason for hiding this comment

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

See line 87 above in the importer class, there is a lot of dict/key usage, often several levels like:
for vendor in data["affects"]["vendor"]["vendor_data"] and then again for products in vendor["product"]["product_data"].

Shouldn't we probably use something like the AdvisoryData classes (or a special subclass for that) which would model the advisory data directly, and then use those classes everywhere instead of dicts. This has some advantages:

  1. code would be more pythonic and readable (and better looking)
  2. we can and should test for the advisory format and handle keyerrors gracefully (idk if the data format changes that much at all, but it could in future?)

Copy link
Member

Choose a reason for hiding this comment

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

Also see https://github.com/nexB/vulnerablecode/pull/1102/files#diff-aa2cf137c50d3fb0242d00e7fbe786cc8e3d8c3d22c32b8afedde439965f2fb9R49, any reason we are using SPDX version of the license expression with spdx_license_expression instead of the scancode license expression version?



ignore_tags = {
IGNORE_TAGS = {
"AGB_BEFORE_AAA_CHANGES",
"APACHE_1_2b1",
"APACHE_1_2b10",
Expand Down Expand Up @@ -209,3 +227,80 @@ def fetch_links(url):
"post_ajp_proxy",
"pre_ajp_proxy",
}


class ApacheHTTPDImprover(Improver):
Copy link
Member

Choose a reason for hiding this comment

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

Why not have these classes as seperate files in the improvers directory? I only see the oval improver has it's own file, is that only because of it's size? We would probably want to be consistent here and have the improvers in their respective directory unless that's inconvenient/not possible because of something.

def __init__(self) -> None:
self.versions_fetcher_by_purl: Mapping[str, VersionAPI] = {}
self.vesions_by_purl = {}

@property
def interesting_advisories(self) -> QuerySet:
return Advisory.objects.filter(created_by=ApacheHTTPDImporter.qualified_name)

def get_package_versions(
self, package_url: PackageURL, until: Optional[datetime] = None
) -> List[str]:
"""
Return a list of `valid_versions` for the `package_url`
"""
api_name = "apache/httpd"
versions_fetcher = GitHubTagsAPI()
return versions_fetcher.get_until(package_name=api_name, until=until).valid_versions

def get_inferences(self, advisory_data: AdvisoryData) -> Iterable[Inference]:
"""
Yield Inferences for the given advisory data
"""
if not advisory_data.affected_packages:
return
try:
purl, affected_version_ranges, _ = AffectedPackage.merge(
advisory_data.affected_packages
)
except UnMergeablePackageError:
logger.error(f"Cannot merge with different purls {advisory_data.affected_packages!r}")
return iter([])

pkg_type = purl.type
pkg_namespace = purl.namespace
pkg_name = purl.name

if not self.vesions_by_purl.get(str(purl)):
valid_versions = self.get_package_versions(
package_url=purl, until=advisory_data.date_published
)
self.vesions_by_purl[str(purl)] = valid_versions

valid_versions = self.vesions_by_purl[str(purl)]

for affected_version_range in affected_version_ranges:
aff_vers, unaff_vers = resolve_version_range(
affected_version_range=affected_version_range,
package_versions=valid_versions,
ignorable_versions=IGNORE_TAGS,
)
affected_purls = [
PackageURL(type=pkg_type, namespace=pkg_namespace, name=pkg_name, version=version)
for version in aff_vers
]

unaffected_purls = [
PackageURL(type=pkg_type, namespace=pkg_namespace, name=pkg_name, version=version)
for version in unaff_vers
]

affected_packages: List[LegacyAffectedPackage] = nearest_patched_package(
vulnerable_packages=affected_purls, resolved_packages=unaffected_purls
)

for (
fixed_package,
affected_packages,
) in get_affected_packages_by_patched_package(affected_packages).items():
yield Inference.from_advisory_data(
advisory_data,
confidence=100, # We are getting all valid versions to get this inference
affected_purls=affected_packages,
fixed_purl=fixed_package,
)
1 change: 1 addition & 0 deletions vulnerabilities/improvers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
importers.istio.IstioImprover,
oval.DebianOvalBasicImprover,
oval.UbuntuOvalBasicImprover,
importers.apache_httpd.ApacheHTTPDImprover,
]

IMPROVERS_REGISTRY = {x.qualified_name: x for x in IMPROVERS_REGISTRY}
25 changes: 25 additions & 0 deletions vulnerabilities/tests/test_apache_httpd.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,17 @@

import json
import os
from unittest import mock

import pytest
from univers.version_constraint import VersionConstraint
from univers.version_range import ApacheVersionRange
from univers.versions import SemverVersion

from vulnerabilities.importer import AdvisoryData
from vulnerabilities.importers.apache_httpd import ApacheHTTPDImporter
from vulnerabilities.importers.apache_httpd import ApacheHTTPDImprover
from vulnerabilities.improvers.default import DefaultImprover
from vulnerabilities.tests import util_tests

BASE_DIR = os.path.dirname(os.path.abspath(__file__))
Expand Down Expand Up @@ -116,3 +120,24 @@ def test_to_advisory_CVE_2022_28614():
result = advisories.to_dict()
expected_file = os.path.join(TEST_DATA, f"CVE-2022-28614-apache-httpd-expected.json")
util_tests.check_results_against_json(result, expected_file)


@mock.patch("vulnerabilities.importers.apache_httpd.ApacheHTTPDImprover.get_package_versions")
def test_apache_httpd_improver(mock_response):
advisory_file = os.path.join(TEST_DATA, f"CVE-2021-44224-apache-httpd-expected.json")
expected_file = os.path.join(TEST_DATA, f"apache-httpd-improver-expected.json")
with open(advisory_file) as exp:
advisory = AdvisoryData.from_dict(json.load(exp))
mock_response.return_value = [
"2.4.8",
"2.4.9",
"2.4.10",
"2.4.53",
"2.4.54",
]
improvers = [ApacheHTTPDImprover(), DefaultImprover()]
result = []
for improver in improvers:
inference = [data.to_dict() for data in improver.get_inferences(advisory)]
result.extend(inference)
util_tests.check_results_against_json(result, expected_file)
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
[
{
"vulnerability_id": null,
"aliases": [
"CVE-2021-44224"
],
"confidence": 100,
"summary": "A crafted URI sent to httpd configured as a forward proxy (ProxyRequests on) can cause a crash (NULL pointer dereference) or, for configurations mixing forward and reverse proxy declarations, can allow for requests to be directed to a declared Unix Domain Socket endpoint (Server Side Request Forgery).\n\nThis issue affects Apache HTTP Server 2.4.7 up to 2.4.51 (included).",
"affected_purls": [
{
"type": "apache",
"namespace": null,
"name": "httpd",
"version": "2.4.8",
"qualifiers": null,
"subpath": null
},
{
"type": "apache",
"namespace": null,
"name": "httpd",
"version": "2.4.9",
"qualifiers": null,
"subpath": null
},
{
"type": "apache",
"namespace": null,
"name": "httpd",
"version": "2.4.10",
"qualifiers": null,
"subpath": null
}
],
"fixed_purl": {
"type": "apache",
"namespace": null,
"name": "httpd",
"version": "2.4.53",
"qualifiers": null,
"subpath": null
},
"references": [
{
"reference_id": "CVE-2021-44224",
"url": "https://httpd.apache.org/security/json/CVE-2021-44224.json",
"severities": [
{
"system": "apache_httpd",
"value": "moderate",
"scoring_elements": ""
}
]
}
],
"weaknesses": []
},
{
"vulnerability_id": null,
"aliases": [
"CVE-2021-44224"
],
"confidence": 100,
"summary": "A crafted URI sent to httpd configured as a forward proxy (ProxyRequests on) can cause a crash (NULL pointer dereference) or, for configurations mixing forward and reverse proxy declarations, can allow for requests to be directed to a declared Unix Domain Socket endpoint (Server Side Request Forgery).\n\nThis issue affects Apache HTTP Server 2.4.7 up to 2.4.51 (included).",
"affected_purls": [
{
"type": "apache",
"namespace": null,
"name": "httpd",
"version": "2.4.7",
"qualifiers": null,
"subpath": null
},
{
"type": "apache",
"namespace": null,
"name": "httpd",
"version": "2.4.51",
"qualifiers": null,
"subpath": null
}
],
"fixed_purl": {
"type": "apache",
"namespace": null,
"name": "httpd",
"version": "2.4.52",
"qualifiers": null,
"subpath": null
},
"references": [
{
"reference_id": "CVE-2021-44224",
"url": "https://httpd.apache.org/security/json/CVE-2021-44224.json",
"severities": [
{
"system": "apache_httpd",
"value": "moderate",
"scoring_elements": ""
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we have a null/none instead of a empty string when there is no value? This wouldn't matter because if "": would produce the same as if None: but it's good practice AFAIK :P

}
]
}
],
"weaknesses": []
}
]