SONARJAVA-6783: Implement rule S9345 Classes with throwing constructors should be protected against Finalizer attacks

[romainbrenguier]

Summary

• Implement new rule S9345 that detects classes with throwing constructors vulnerable to Finalizer attacks
• Non-compliant: non-final class with public/protected/package-private constructor that has a throws clause or contains throw statements
• Compliant: final classes, classes with only private throwing constructors (factory pattern), enums, records, sealed classes (where all permitted subclasses are final/sealed), classes with final finalize() method, and local classes

Review feedback addressed (commit 09126f5)

All 6 findings from @nathsou have been addressed:

• [P1] Handle instance/field initializers and classes with no explicit constructor
• [P1] Abstract classes are no longer exempt (attacker can subclass)
• [P1] Sealed classes with non-sealed subclasses are correctly flagged
• [P2] Classes with protected final void finalize() {} recognized as safe
• [P2] Removed inaccurate CWE-586 mapping, kept CERT OBJ11-J reference
• [P3] Local classes are excluded (cannot be subclassed from outside)

Test plan

• Unit test with CheckVerifier covering noncompliant and compliant patterns
• Test without semantic analysis
• Ruling results updated
• CI passes

[hashicorp-vault-sonar-prod]

SONARJAVA-6783

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #5982

Please review and merge it into your branch.

[github-actions]

Ruling Diff Summary

Detected changes in 5 rule files: 0 issues removed, 149 issues added.

S9345 (java) on commons-beanutils - 0 issues removed, 77 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/BaseDynaBeanMapDecorator.java (line 78)

        73 |      * @param dynaBean The dyna bean being decorated
        74 |      * @param readOnly <code>true</code> if the Map is read only
        75 |      * otherwise <code>false</code>
        76 |      * @throws IllegalArgumentException if the {@link DynaBean} is null.
        77 |      */
>>>     78 |     public BaseDynaBeanMapDecorator(final DynaBean dynaBean, final boolean readOnly) {
        79 |         if (dynaBean == null) {
        80 |             throw new IllegalArgumentException("DynaBean is null");
        81 |         }
        82 |         this.dynaBean = dynaBean;
        83 |         this.readOnly = readOnly;

Added src/main/java/org/apache/commons/beanutils2/BeanPredicate.java (line 49)

        44 |      * @param propertyName the name of the property whose value is to be predicated,
        45 |      * not null
        46 |      * @param predicate the <code>Predicate</code> to be applied,
        47 |      * not null
        48 |      */
>>>     49 |     public BeanPredicate(final String propertyName, final Predicate predicate) {
        50 |         this.propertyName = propertyName;
        51 |         this.predicate = predicate;
        52 |     }
        53 | 
        54 |     /**

Added src/main/java/org/apache/commons/beanutils2/BeanPropertyValueChangeClosure.java (line 117)

       112 |      * <code>propertyValue</code>.
       113 |      * @param propertyValue The value that <code>propertyName</code> will be set to on the target
       114 |      * object.
       115 |      * @throws IllegalArgumentException If the propertyName provided is null or empty.
       116 |      */
>>>    117 |     public BeanPropertyValueChangeClosure(final String propertyName, final Object propertyValue) {
       118 |         this(propertyName, propertyValue, false);
       119 |     }
       120 | 
       121 |     /**
       122 |      * Constructor which takes the name of the property to be changed, the new value to set

Added src/main/java/org/apache/commons/beanutils2/BeanPropertyValueChangeClosure.java (line 134)

       129 |      * object.
       130 |      * @param ignoreNull Determines whether <code>null</code> objects in the property path will
       131 |      * genenerate an <code>IllegalArgumentException</code> or not.
       132 |      * @throws IllegalArgumentException If the propertyName provided is null or empty.
       133 |      */
>>>    134 |     public BeanPropertyValueChangeClosure(final String propertyName, final Object propertyValue, final boolean ignoreNull) {
       135 |         super();
       136 | 
       137 |         if (propertyName != null && propertyName.length() > 0) {
       138 |             this.propertyName = propertyName;
       139 |             this.propertyValue = propertyValue;

Added src/main/java/org/apache/commons/beanutils2/BeanPropertyValueEqualsPredicate.java (line 149)

       144 |      *
       145 |      * @param propertyName The name of the property that will be evaluated against the expected value.
       146 |      * @param propertyValue The value to use in object evaluation.
       147 |      * @throws IllegalArgumentException If the property name provided is null or empty.
       148 |      */
>>>    149 |     public BeanPropertyValueEqualsPredicate(final String propertyName, final Object propertyValue) {
       150 |         this(propertyName, propertyValue, false);
       151 |     }
       152 | 
       153 |     /**
       154 |      * Constructor which takes the name of the property, its expected value

Added src/main/java/org/apache/commons/beanutils2/BeanPropertyValueEqualsPredicate.java (line 164)

       159 |      * @param propertyValue The value to use in object evaluation.
       160 |      * @param ignoreNull Determines whether <code>null</code> objects in the property path will
       161 |      * genenerate an <code>IllegalArgumentException</code> or not.
       162 |      * @throws IllegalArgumentException If the property name provided is null or empty.
       163 |      */
>>>    164 |     public BeanPropertyValueEqualsPredicate(final String propertyName, final Object propertyValue, final boolean ignoreNull) {
       165 |         super();
       166 | 
       167 |         if (propertyName != null && propertyName.length() > 0) {
       168 |             this.propertyName = propertyName;
       169 |             this.propertyValue = propertyValue;

Added src/main/java/org/apache/commons/beanutils2/BeanToPropertyValueTransformer.java (line 103)

        98 |      *
        99 |      * @param propertyName The name of the property that will be used in the transformation.
       100 |      * @throws IllegalArgumentException If the <code>propertyName</code> is <code>null</code> or
       101 |      * empty.
       102 |      */
>>>    103 |     public BeanToPropertyValueTransformer(final String propertyName) {
       104 |         this(propertyName, false);
       105 |     }
       106 | 
       107 |     /**
       108 |      * Constructs a Transformer and sets ignoreNull.

Added src/main/java/org/apache/commons/beanutils2/BeanToPropertyValueTransformer.java (line 119)

       114 |      * @param ignoreNull Determines whether <code>null</code> objects in the property path will
       115 |      * genenerate an <code>IllegalArgumentException</code> or not.
       116 |      * @throws IllegalArgumentException If the <code>propertyName</code> is <code>null</code> or
       117 |      * empty.
       118 |      */
>>>    119 |     public BeanToPropertyValueTransformer(final String propertyName, final boolean ignoreNull) {
       120 |         super();
       121 | 
       122 |         if (propertyName != null && propertyName.length() > 0) {
       123 |             this.propertyName = propertyName;
       124 |             this.ignoreNull = ignoreNull;

Added src/main/java/org/apache/commons/beanutils2/BeanUtilsBean.java (line 111)

       106 | 
       107 |     /**
       108 |      * <p>Constructs an instance using new property
       109 |      * and conversion instances.</p>
       110 |      */
>>>    111 |     public BeanUtilsBean() {
       112 |         this(new ConvertUtilsBean(), new PropertyUtilsBean());
       113 |     }
       114 | 
       115 |     /**
       116 |      * <p>Constructs an instance using given conversion instances

Added src/main/java/org/apache/commons/beanutils2/BeanUtilsBean.java (line 124)

       119 |      * @param convertUtilsBean use this <code>ConvertUtilsBean</code>
       120 |      * to perform conversions from one object to another
       121 |      *
       122 |      * @since 1.8.0
       123 |      */
>>>    124 |     public BeanUtilsBean(final ConvertUtilsBean convertUtilsBean) {
       125 |         this(convertUtilsBean, new PropertyUtilsBean());
       126 |     }
       127 | 
       128 |     /**
       129 |      * <p>Constructs an instance using given property and conversion instances.</p>

Added src/main/java/org/apache/commons/beanutils2/BeanUtilsBean.java (line 136)

       131 |      * @param convertUtilsBean use this <code>ConvertUtilsBean</code>
       132 |      * to perform conversions from one object to another
       133 |      * @param propertyUtilsBean use this <code>PropertyUtilsBean</code>
       134 |      * to access properties
       135 |      */
>>>    136 |     public BeanUtilsBean(
       137 |                             final ConvertUtilsBean convertUtilsBean,
       138 |                             final PropertyUtilsBean propertyUtilsBean) {
       139 | 
       140 |         this.convertUtilsBean = convertUtilsBean;
       141 |         this.propertyUtilsBean = propertyUtilsBean;

Added src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java (line 158)

       153 |     private final Log log = LogFactory.getLog(ConvertUtilsBean.class);
       154 | 
       155 |     // ------------------------------------------------------- Constructors
       156 | 
       157 |     /** Construct a bean with standard converters registered */
>>>    158 |     public ConvertUtilsBean() {
       159 |         converters.setFast(false);
       160 |         deregister();
       161 |         converters.setFast(true);
       162 |     }
       163 | 

Added src/main/java/org/apache/commons/beanutils2/FluentPropertyBeanIntrospector.java (line 97)

        92 |      * be inspected.
        93 |      *
        94 |      * @param writePrefix the prefix for write methods (must not be <b>null</b>)
        95 |      * @throws IllegalArgumentException if the prefix is <b>null</b>
        96 |      */
>>>     97 |     public FluentPropertyBeanIntrospector(final String writePrefix) {
        98 |         if (writePrefix == null) {
        99 |             throw new IllegalArgumentException(
       100 |                     "Prefix for write methods must not be null!");
       101 |         }
       102 |         writeMethodPrefix = writePrefix;

Added src/main/java/org/apache/commons/beanutils2/FluentPropertyBeanIntrospector.java (line 110)

       105 |     /**
       106 |      *
       107 |      * Creates a new instance of <code>FluentPropertyBeanIntrospector</code> and
       108 |      * sets the default prefix for write methods.
       109 |      */
>>>    110 |     public FluentPropertyBeanIntrospector() {
       111 |         this(DEFAULT_WRITE_METHOD_PREFIX);
       112 |     }
       113 | 
       114 |     /**
       115 |      * Returns the prefix for write methods this instance scans for.

Added src/main/java/org/apache/commons/beanutils2/LazyDynaBean.java (line 172)

       167 |     // ------------------- Constructors ----------------------------------
       168 | 
       169 |     /**
       170 |      * Construct a new <code>LazyDynaBean</code> with a <code>LazyDynaClass</code> instance.
       171 |      */
>>>    172 |     public LazyDynaBean() {
       173 |         this(new LazyDynaClass());
       174 |     }
       175 | 
       176 |     /**
       177 |      * Construct a new <code>LazyDynaBean</code> with a <code>LazyDynaClass</code> instance.

S9345 (java) on eclipse-jetty - 0 issues removed, 51 issues added - new ruling file

Added jetty-http/src/main/java/org/eclipse/jetty/http/HostPortHttpField.java (line 37)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/HostPortHttpField.java)

Added jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java (line 125)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java)

Added jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java (line 36)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java)

Added jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java (line 76)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java)

Added jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java (line 51)

(source file not found at this revision: jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java)

Added jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java (line 90)

(source file not found at this revision: jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java (line 303)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java (line 68)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/EncodingHttpWriter.java (line 33)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/EncodingHttpWriter.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelListeners.java (line 54)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelListeners.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java (line 370)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/ServletPathMapping.java (line 47)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/ServletPathMapping.java)

Added jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/JSONPojoConvertorFactory.java (line 46)

(source file not found at this revision: jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/JSONPojoConvertorFactory.java)

Added jetty-util/src/main/java/org/eclipse/jetty/util/BlockingArrayQueue.java (line 126)

(source file not found at this revision: jetty-util/src/main/java/org/eclipse/jetty/util/BlockingArrayQueue.java)

Added jetty-util/src/main/java/org/eclipse/jetty/util/ClassLoadingObjectInputStream.java (line 48)

(source file not found at this revision: jetty-util/src/main/java/org/eclipse/jetty/util/ClassLoadingObjectInputStream.java)

S9345 (java) on eclipse-jetty-similar-to-main - 0 issues removed, 12 issues added - new ruling file

Added jetty-http/src/main/java/org/eclipse/jetty/http/HostPortHttpField.java (line 37)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/HostPortHttpField.java)

Added jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java (line 125)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java)

Added jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java (line 36)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java)

Added jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java (line 76)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java)

Added jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java (line 51)

(source file not found at this revision: jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java)

Added jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java (line 90)

(source file not found at this revision: jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java (line 303)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java (line 68)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/EncodingHttpWriter.java (line 33)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/EncodingHttpWriter.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelListeners.java (line 54)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelListeners.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java (line 370)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java)

Added jetty-server/src/main/java/org/eclipse/jetty/server/ServletPathMapping.java (line 47)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/ServletPathMapping.java)

S9345 (java) on guava - 0 issues removed, 2 issues added - new ruling file

Added src/com/google/common/base/FinalizableReferenceQueue.java (line 159)

       154 |   final boolean threadStarted;
       155 | 
       156 |   /**
       157 |    * Constructs a new queue.
       158 |    */
>>>    159 |   public FinalizableReferenceQueue() {
       160 |     // We could start the finalizer lazily, but I'd rather it blow up early.
       161 |     queue = new ReferenceQueue<Object>();
       162 |     frqRef = new PhantomReference<Object>(this, queue);
       163 |     boolean threadStarted = false;
       164 |     try {

Added src/com/google/common/io/MultiReader.java (line 37)

        32 |  */
        33 | class MultiReader extends Reader {
        34 |   private final Iterator<? extends CharSource> it;
        35 |   private Reader current;
        36 | 
>>>     37 |   MultiReader(Iterator<? extends CharSource> readers) throws IOException {
        38 |     this.it = readers;
        39 |     advance();
        40 |   }
        41 | 
        42 |   /**

S9345 (java) on sonar-server - 0 issues removed, 7 issues added - new ruling file

Added src/main/java/org/sonar/server/computation/task/projectanalysis/source/ReportIterator.java (line 38)

(source file not found at this revision: src/main/java/org/sonar/server/computation/task/projectanalysis/source/ReportIterator.java)

Added src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java (line 114)

(source file not found at this revision: src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java)

Added src/main/java/org/sonar/server/platform/web/MasterServletFilter.java (line 48)

(source file not found at this revision: src/main/java/org/sonar/server/platform/web/MasterServletFilter.java)

Added src/main/java/org/sonar/server/plugins/UpdateCenterClient.java (line 75)

(source file not found at this revision: src/main/java/org/sonar/server/plugins/UpdateCenterClient.java)

Added src/main/java/org/sonar/server/user/SecurityRealmFactory.java (line 43)

(source file not found at this revision: src/main/java/org/sonar/server/user/SecurityRealmFactory.java)

Added src/main/java/org/sonar/server/util/ObjectInputStreamIterator.java (line 35)

(source file not found at this revision: src/main/java/org/sonar/server/util/ObjectInputStreamIterator.java)

Added src/main/java/org/sonar/server/util/cache/DiskCache.java (line 42)

(source file not found at this revision: src/main/java/org/sonar/server/util/cache/DiskCache.java)

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #5983

Please review and merge it into your branch.

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #5983

Please review and merge it into your branch.

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #5983

Please review and merge it into your branch.

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6000

Please review and merge it into your branch.

[nathsou]

Changes requested. See the inline comments for the six findings; the initializer-path issue is also tracked in the existing unresolved thread.

[nathsou]

Inline findings for the requested changes.

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6007

Please review and merge it into your branch.

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6010

Please review and merge it into your branch.

[nathsou]

Single-pass re-review of the follow-up commits.

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6031

Please review and merge it into your branch.

[gitar-bot]

Code Review ✅ Approved 3 resolved / 3 findings

Implements rule S9345 to detect classes with throwing constructors vulnerable to Finalizer attacks, addressing issues with sealed classes, throwing initializers, and extraneous ruling results.

✅ 3 resolved

✅ Edge Case: Sealed classes flagged as noncompliant (false positive)

📄 java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java:45-48
A sealed non-final class cannot be subclassed by an attacker — its permitted subclasses are fixed at compile time — so it is not exploitable by a Finalizer attack, yet the check only exempts final and abstract classes and would report a sealed class with a throwing constructor. Consider also returning early when ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.SEALED) is true (subject to RSPEC scope), to avoid false positives on sealed hierarchies.

✅ Edge Case: Throwing field/instance initializers not detected

📄 java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java:49-63
The check only inspects CONSTRUCTOR members and scans each constructor's own block, so a throw that occurs in an instance initializer block or a field initializer (e.g. private final X x = compute(); where compute() throws) is missed even though those run during construction. A class with no explicit constructor but a throwing field/instance initializer has no CONSTRUCTOR member at all and is never flagged, a false negative for the same finalizer-attack vector. Consider also examining instance initializer blocks and field initializers, and accounting for the implicit default constructor.

✅ Quality: Unrelated ruling results (S2160, S6212) bundled into new-rule PR

📄 its/ruling/src/test/resources/mall/java-S2160.json:1-4 📄 its/ruling/src/test/resources/mall/java-S6212.json:13
This PR adds rule S9345, but it also introduces a new S2160 expected-results file and adds line 91 to the S6212 results for the mall project. These rules are unrelated to the Finalizer-attack rule and the ruling projects (mall, guava, etc.) are unaffected by the test-source and check additions here, so these diffs likely reflect stale/regenerated results being swept in rather than an intended change. Bundling unrelated rule-result churn into a feature PR obscures whether it hides a real regression; either split these out into a dedicated ruling-update PR or confirm they are expected and intentional.

Implementation Status ✅ 1 of 1 objectives covered

✅ SONARJAVA-6783 - 1 of 1 objectives covered

This PR implements rule S9345 to protect classes with throwing constructors against Finalizer attacks.

✅ 1 covered here

• ✅ Implement rule S9345 to protect classes with throwing constructors against Finalizer attacks

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
90.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube