Skip to content

Add comprehensive CVE data validation to prevent processing crashes#9

Open
lochlanmcelroy wants to merge 1 commit intocisagov:developfrom
lochlanmcelroy:develop
Open

Add comprehensive CVE data validation to prevent processing crashes#9
lochlanmcelroy wants to merge 1 commit intocisagov:developfrom
lochlanmcelroy:develop

Conversation

@lochlanmcelroy
Copy link
Copy Markdown

@lochlanmcelroy lochlanmcelroy commented Aug 1, 2025

Add comprehensive CVE data validation to prevent processing crashes

🗣 Description

This PR adds robust input validation to the process_cve_json function to handle malformed or unexpected CVE data from the NVD. The validation includes type checking for JSON structure, CVSS score range validation (0.0-10.0), and comprehensive error handling with detailed logging.

💭 Motivation and context

The current code has minimal validation and could crash or corrupt data if:

  • NVD changes their JSON structure unexpectedly
  • Network issues cause truncated or malformed JSON responses
  • CVSS scores contain invalid values (negative numbers, strings, values > 10.0)
  • CVE items have unexpected data types or missing required fields

This change makes the CVE sync process more robust by validating data before processing and gracefully skipping invalid entries while logging detailed warnings for debugging.

🧪 Testing

Enhanced the existing process_cve_json function with additional validation logic. The changes maintain backward compatibility and can be tested with existing test suite. Manual testing shows proper handling of invalid data with appropriate logging.

I ran some local tests for formatting & edge cases to ensure everything worked smoothly, and can add those to the repo as well if needed.

All existing tests continue to pass, ensuring no breaking changes to current functionality.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All future TODOs are captured in issues, which are referenced in code comments.
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated to reflect the changes in this PR.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.
  • Bump major, minor, patch, pre-release, and/or build versions as appropriate via the bump_version script if this repository is versioned and the changes in this PR warrant a version bump.
  • Create a pre-release (necessary if and only if the pre-release version was bumped).

✅ Pre-merge checklist

  • Revert dependencies to default branches.
  • Finalize version.

✅ Post-merge checklist

  • Create a release (necessary if and only if the version was

@lochlanmcelroy
Copy link
Copy Markdown
Author

I don't think I can add labels & some of the checklist is tough for me to do alone, but I hope this is a helpful first contribution :)

@coveralls
Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 16666400260

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 6 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-2.7%) to 97.345%

Files with Coverage Reduction New Missed Lines %
cve_sync.py 6 96.05%
Totals Coverage Status
Change from base Build 16658114853: -2.7%
Covered Lines: 179
Relevant Lines: 185

💛 - Coveralls

@dav3r dav3r added the improvement This issue or pull request will add or improve functionality, maintainability, or ease of use label Aug 1, 2025
@dav3r dav3r added this to CyHy System Aug 1, 2025
@dav3r dav3r moved this to In Progress in CyHy System Aug 1, 2025
@jsf9k jsf9k assigned jsf9k and dav3r and unassigned jsf9k Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement This issue or pull request will add or improve functionality, maintainability, or ease of use

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

4 participants