Skip to content

Set CWE-134 from 9.3 to 7.3 CVSS score for memory safe languages #19530

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

felickz
Copy link
Contributor

@felickz felickz commented May 19, 2025

This pull request adjusts the @security-severity level for several queries related to uncontrolled format strings across memory safe languages to better reflect their impact. The risk is limited to application crashes or information disclosure, not system compromise. The severity has been reduced from 9.3 to 7.3 to match up to the similar Javascript / Ruby queries.

- Sync up to score given to javascript/ruby
@Copilot Copilot AI review requested due to automatic review settings May 19, 2025 18:45
@felickz felickz requested review from a team as code owners May 19, 2025 18:45
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR aligns the @security-severity rating for uncontrolled format string queries in memory-safe languages with existing JavaScript/Ruby rules by lowering it from 9.3 to 7.3.

  • Reduced severity score in Swift, Java, and C# queries to 7.3
  • Ensures consistency across supported memory-safe languages

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
swift/ql/src/queries/Security/CWE-134/UncontrolledFormatString.ql Updated @security-severity from 9.3 to 7.3
java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatString.ql Updated @security-severity from 9.3 to 7.3
csharp/ql/src/Security Features/CWE-134/UncontrolledFormatString.ql Updated @security-severity from 9.3 to 7.3

michaelnebel
michaelnebel previously approved these changes May 20, 2025
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

C# LGTM

owen-mc
owen-mc previously approved these changes May 20, 2025
Copy link
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

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

Java 👍🏻

geoffw0
geoffw0 previously approved these changes May 21, 2025
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

Swift 👍

@felickz
Copy link
Contributor Author

felickz commented May 21, 2025

Worth a changenote here or no-change-note-required This PR does not need a change note ? (seeing precedent that other similar changes did not have a note)

@owen-mc
Copy link
Contributor

owen-mc commented May 21, 2025

It should have a change note. There is a guide here. It should be a query change note, with the category "queryMetadata".

@felickz felickz dismissed stale reviews from michaelnebel, geoffw0, and owen-mc via 53a6133 June 6, 2025 16:24
@felickz
Copy link
Contributor Author

felickz commented Jun 6, 2025

It should have a change note. There is a guide here. It should be a query change note, with the category "queryMetadata".

Change notes added 🙇

@felickz felickz requested a review from owen-mc June 6, 2025 16:27
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

C# LGTM

Copy link
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

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

Java LGTM

@owen-mc
Copy link
Contributor

owen-mc commented Jun 10, 2025

I don't know why the frameworks coverage artifacts CI job failed. I've restarted it. If it runs successfully then it won't find anything interesting for this PR, so if it fails again I don't think it should block this PR.

@jketema
Copy link
Contributor

jketema commented Jun 10, 2025

I don't know why the frameworks coverage artifacts CI job failed. I've restarted it. If it runs successfully then it won't find anything interesting for this PR, so if it fails again I don't think it should block this PR.

It's complaining about something related to the PR though. Are the line endings of the Swift change note Unix line endings?

Error is:

error: Your local changes to the following files would be overwritten by checkout:
	swift/ql/src/change-notes/2025-06-06-reduce-CWE-134-for-memory-safe-languages.md
Please commit your changes or stash them before you switch branches.
Aborting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants