Copilot commented on code in PR #4341:
URL:
https://github.com/apache/incubator-kie-kogito-runtimes/pull/4341#discussion_r3578642984
##########
quarkus/extensions/kogito-quarkus-rules-extension/kogito-quarkus-rules/src/main/java/org/kie/kogito/core/rules/incubation/quarkus/support/RuleUnitServiceImpl.java:
##########
@@ -36,6 +37,9 @@
class RuleUnitServiceImpl implements RuleUnitService {
+ // CWE-470: validate class name is a legal Java FQCN before passing to
loadClass
+ private static final Pattern SAFE_CLASS_NAME =
Pattern.compile("[\\w$]+(\\.[\\w$]+)*");
Review Comment:
SAFE_CLASS_NAME is described as validating a legal Java fully-qualified
class name, but the current regex allows identifier segments starting with
digits (e.g., "1abc"), which is not a legal Java identifier start. Consider
using Java identifier character classes to correctly model Java binary names
(including $ for inner classes).
##########
quarkus/extensions/kogito-quarkus-rules-extension/kogito-quarkus-rules/src/main/java/org/kie/kogito/core/rules/incubation/quarkus/support/StatefulRuleUnitServiceImpl.java:
##########
@@ -41,6 +42,9 @@
class StatefulRuleUnitServiceImpl implements StatefulRuleUnitService {
+ // CWE-470: validate class name is a legal Java FQCN before passing to
loadClass
+ private static final Pattern SAFE_CLASS_NAME =
Pattern.compile("[\\w$]+(\\.[\\w$]+)*");
Review Comment:
SAFE_CLASS_NAME is intended to validate a legal Java fully-qualified class
name, but the current regex permits segments starting with digits, which Java
does not allow. Using Java identifier character classes avoids accepting
invalid binary names while still rejecting injection characters.
##########
quarkus/extensions/kogito-quarkus-rules-extension/kogito-quarkus-rules/src/main/java/org/kie/kogito/core/rules/incubation/quarkus/support/RuleUnitServiceImpl.java:
##########
@@ -64,12 +68,16 @@ public Stream<DataContext> evaluate(Id id, DataContext
inputContext) {
}
private RuleUnitData convertValue(Map<String, Object> payload, RuleUnitId
ruleUnitId) {
+ String className = ruleUnitId.ruleUnitId();
+ if (!SAFE_CLASS_NAME.matcher(className).matches()) {
+ throw new IllegalArgumentException("Invalid rule unit class name:
" + className);
+ }
Review Comment:
This PR validates RuleUnitId before loadClass() in
RuleUnitServiceImpl/StatefulRuleUnitServiceImpl, but the same module still has
an unvalidated loadClass(ruleUnitId.ruleUnitId()) in
DataSourceServiceImpl#toClass
(kogito-quarkus-rules/.../DataSourceServiceImpl.java:129-135). If the goal is
to remediate CWE-470 for this extension, that call path should be updated to
apply the same validation (ideally via a shared helper) so the vulnerability
isn't still reachable via data source operations.
##########
quarkus/extensions/kogito-quarkus-rules-extension/kogito-quarkus-rules/src/main/java/org/kie/kogito/core/rules/incubation/quarkus/support/StatefulRuleUnitServiceImpl.java:
##########
@@ -71,8 +75,12 @@ public MetaDataContext create(LocalId localId,
ExtendedReferenceContext extended
}
private Class<RuleUnitData> toClass(RuleUnitId ruleUnitId) {
+ String className = ruleUnitId.ruleUnitId();
+ if (!SAFE_CLASS_NAME.matcher(className).matches()) {
+ throw new IllegalArgumentException("Invalid rule unit class name:
" + className);
+ }
try {
- return (Class<RuleUnitData>)
Thread.currentThread().getContextClassLoader().loadClass(ruleUnitId.ruleUnitId());
+ return (Class<RuleUnitData>)
Thread.currentThread().getContextClassLoader().loadClass(className);
} catch (ClassNotFoundException e) {
throw new IllegalArgumentException(e);
}
Review Comment:
When loadClass fails, the thrown IllegalArgumentException drops useful
context (which class name failed to load). Including the class name (as
RuleUnitServiceImpl does) will make this error actionable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
