Skip to content

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

Closed
felickz wants to merge 5 commits into
github:mainfrom
forks-felickz:cwe-134
Closed

Set CWE-134 from 9.3 to 7.3 CVSS score for memory safe languages#19530
felickz wants to merge 5 commits into
github:mainfrom
forks-felickz:cwe-134

Conversation

@felickz

@felickzfelickz commented May 19, 2025

Copy link
Copy Markdown
Contributor

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
CopilotAI review requested due to automatic review settings May 19, 2025 18:45
@felickz
felickz requested review from a team as code ownersMay 19, 2025 18:45

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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.

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

michaelnebel
michaelnebel previously approved these changes May 20, 2025

@michaelnebelmichaelnebel left a comment

Copy link
Copy Markdown
Contributor

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

@owen-mcowen-mc left a comment

Copy link
Copy Markdown
Contributor

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

@geoffw0geoffw0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Swift 👍

@felickz

Copy link
Copy Markdown
ContributorAuthor

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

@owen-mc

Copy link
Copy Markdown
Contributor

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

@felickz

Copy link
Copy Markdown
ContributorAuthor

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-mcJune 6, 2025 16:27
michaelnebel
michaelnebel previously approved these changes Jun 10, 2025

@michaelnebelmichaelnebel left a comment

Copy link
Copy Markdown
Contributor

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 Jun 10, 2025

@owen-mcowen-mc left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor

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

jketema commented Jun 10, 2025

Copy link
Copy Markdown
Contributor

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

@felickz
felickz dismissed stale reviews from owen-mc and michaelnebel via 00e9c38June 11, 2025 17:06
@owen-mc

Copy link
Copy Markdown
Contributor

I've made another PR which is just a copy of this one with the problem fixed. I suggest we merge that one instead and close this one.

@owen-mcowen-mc closed this Jun 12, 2025
Sign up for freeto 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.

6 participants

@felickz@owen-mc@jketema@michaelnebel@geoffw0