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

Java: Add new quality query to detect finalize calls #19075

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

jcogs33
Copy link
Contributor

@jcogs33 jcogs33 commented Mar 20, 2025

Description

Adds a new quality query to detect finalize calls. This query is migrated from the advance security team's quality queries.

Consideration

Changes from original query. Let me know if you disagree with any of these changes:

  • The query only alerts on finalize calls now. Alerts for runFinalizersOnExit and gc will continue to be reported on the pre-existing queries java/run-finalizers-on-exit and java/garbage-collection, respectively. I will make follow-up PRs updating these pre-existing queries if needed.
  • Added an exclusion for super.finalize calls that occur within a callable that overrides finalize since the Java doc for finalize states: "If a subclass overrides finalize it must invoke the superclass finalizer explicitly", and alerting on these calls would contradict our pre-existing java/missing-super-finalize query. This exclusion reduces the number of alerts on the MRVA top-100 from 66 to 24 and on the MRVA top-1000 from 954 to 115.
  • Added an exclusion for calls to overloads of finalize. My understanding is that the JVM will not call overloads of finalize when handling garbage collection, but will only call finalize() and overrides of finalize(). So I don't think overloads are relevant for alerts. Let me know if you disagree. This exclusion further reduces the number of alerts on the MRVA top-100 from 24 to 5 and on the MRVA top-1000 from 115 to 40.

Other Notes:

  • TODO: Autofix validation note
  • DCA looks good

@jcogs33 jcogs33 force-pushed the jcogs33/java/do-not-use-finalizers branch from 815802f to 90653ed Compare March 20, 2025 17:40
@jcogs33 jcogs33 force-pushed the jcogs33/java/do-not-use-finalizers branch from 1674c8d to ed22a16 Compare March 27, 2025 23:36
Jami Cogswell added 2 commits March 27, 2025 19:38
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.

None yet

1 participant