Skip to content

Commit fe73af4

Browse files
committed
JEXL-462 : close RESTRICTED permission set; fix wildcard exposure
- Replace all '.*' wildcards in RESTRICTED with explicit '+{}' package declarations so future JDK subpackages are never silently admitted. - Add missing safe packages (java.uti.function/stream/regex/concurrent.atomic, java.time.chrono/format/temporal/zone) and deny known hazards (ZoneRulesProvider, executor classes, java.util.zip/jar/prefs/logging via the closed-world boundary). - Extend the permissions engine: any explicit package declaration (positive or negative) now closes the world — only declared packages are accessible. - NoJexlPackage gains hasAllowedClass() to distinguish a deny-list (unlisted class = allowed) from an allow-list (unlisted class = denied), matching the semantics of 'java.lang { Runtime {} }' vs 'java.io -{ +PrintWriter{} }'.
1 parent 2c900c8 commit fe73af4

3 files changed

Lines changed: 97 additions & 30 deletions

File tree

src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,22 @@ NoJexlClass getNoJexl(final Class<?> clazz) {
172172

173173
boolean isEmpty() { return nojexl.isEmpty(); }
174174

175+
/**
176+
* Whether this package has at least one explicitly-allowed class (a {@code JexlClass} entry).
177+
* <p>A package with allowed-class entries acts as an allow-list: unlisted classes are denied.
178+
* A package with only denied-class entries acts as a deny-list: unlisted classes are allowed.</p>
179+
*
180+
* @return true if at least one class is explicitly allowed
181+
*/
182+
boolean hasAllowedClass() {
183+
for (final NoJexlClass njc : nojexl.values()) {
184+
if (njc instanceof JexlClass) {
185+
return true;
186+
}
187+
}
188+
return false;
189+
}
190+
175191
@Override public NoJexlPackage copy() {
176192
return new NoJexlPackage(copyMap(nojexl));
177193
}
@@ -250,13 +266,18 @@ NoJexlClass getNoJexl(final Class<?> clazz) {
250266
*/
251267
private final Map<String, NoJexlPackage> packages;
252268
/**
253-
* The closed world package patterns.
269+
* The allowed package patterns (wildcards or exact package names).
270+
* <p>Empty together with an empty {@link #packages} map means open-world: every package is accessible
271+
* and only explicitly denied elements are carved out — the behavior of {@link #UNRESTRICTED}.
272+
* Empty with a non-empty {@link #packages} map, or non-empty, means closed-world: only declared
273+
* packages are accessible.</p>
254274
*/
255275
private final Set<String> allowed;
256276

257277
/** Allow inheritance. */
258278
protected Permissions() {
259-
this(Collections.emptySet(), Collections.emptyMap());
279+
this.allowed = Collections.emptySet();
280+
this.packages = Collections.emptyMap();
260281
}
261282

262283
/**
@@ -361,14 +382,29 @@ Set<String> getWildcards() {
361382
return allowed == null ? Collections.emptySet() : Collections.unmodifiableSet(allowed);
362383
}
363384

385+
/**
386+
* Whether a package belongs to the allowed perimeter.
387+
* <p>Open-world ({@link #UNRESTRICTED}: no rules at all) allows every package. Closed-world requires the
388+
* package to match an entry in {@link #allowed}; an empty perimeter in closed-world matches nothing.</p>
389+
*
390+
* @param packageName the package name (not null)
391+
* @return true if allowed, false otherwise
392+
*/
393+
private boolean allowedPackage(final String packageName) {
394+
if (allowed.isEmpty() && packages.isEmpty()) {
395+
return true;
396+
}
397+
return !allowed.isEmpty() && wildcardAllow(allowed, packageName);
398+
}
399+
364400
/**
365401
* Whether the wildcard set of packages allows a given class to be introspected.
366402
*
367403
* @param clazz the package name (not null)
368404
* @return true if allowed, false otherwise
369405
*/
370406
private boolean wildcardAllow(final Class<?> clazz) {
371-
return wildcardAllow(allowed, ClassTool.getPackageName(clazz));
407+
return allowedPackage(ClassTool.getPackageName(clazz));
372408
}
373409

374410
/**
@@ -384,19 +420,21 @@ private boolean wildcardAllow(final Class<?> clazz) {
384420
*/
385421
private <T> boolean specifiedAllow(final Class<?> clazz, T name, BiPredicate<NoJexlClass, T> check) {
386422
final String packageName = ClassTool.getPackageName(clazz);
387-
if (wildcardAllow(allowed, packageName)) {
423+
if (allowedPackage(packageName)) {
388424
return true;
389425
}
390426
final NoJexlPackage njp = packages.get(packageName);
391427
if (njp != null && check != null) {
392428
// there is a package permission, check if there is a class permission
393429
final NoJexlClass njc = njp.getNoJexl(clazz);
394-
// if there is a class permission, perform the check
395430
if (njc != null) {
396431
return check.test(njc, name);
397432
}
433+
// class not listed: allowed if the package is a deny-list (no explicit class allows);
434+
// denied if the package is an allow-list (e.g. java.io -{ +PrintWriter{} ... })
435+
return !njp.hasAllowedClass();
398436
}
399-
// nothing explicit
437+
// package not declared at all
400438
return false;
401439
}
402440

@@ -627,7 +665,7 @@ public boolean allow(final Package pack) {
627665
// an explicit package entry is allowed unless it is the deny marker
628666
final String name = pack.getName();
629667
final NoJexlPackage njp = packages.get(name);
630-
return njp == null ? wildcardAllow(allowed, name) : !Objects.equals(NOJEXL_PACKAGE, njp);
668+
return njp == null ? allowedPackage(name) : !Objects.equals(NOJEXL_PACKAGE, njp);
631669
}
632670

633671

src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,13 @@ private void readPackages() {
381381
packages.put(pname, negative == null || negative
382382
? Permissions.NOJEXL_PACKAGE
383383
: Permissions.JEXL_PACKAGE);
384+
// a wholly-allowed package (pkg +{}) joins the allowed perimeter as an exact match (no '.*').
385+
// This lets a closed set of packages be declared without wildcards - so future sub-packages are
386+
// never implicitly allowed - while still allowing types based on this package (e.g. a foreign
387+
// Map implementation extending java.util.AbstractMap) the same way a '.*' wildcard would.
388+
if (Boolean.FALSE.equals(negative)) {
389+
wildcards.add(pname);
390+
}
384391
} else {
385392
packages.put(pname, njpackage);
386393
}

src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java

Lines changed: 45 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -273,36 +273,58 @@ public JexlPermissions compose(final String... src) {
273273
* and its host.
274274
* </p>
275275
* <p>
276-
* As a simple guide, any line that ends with &quot;.*&quot; is allowing a package, any other is
277-
* denying a package, class or method.
276+
* Every allowed package is declared explicitly using the positive {@code +{}} syntax rather than a
277+
* {@code .*} wildcard. A wildcard matches a package <em>and all of its sub-packages</em>, which is not
278+
* future-proof: a sub-package added by a later JDK (or a dangerous existing one such as
279+
* {@code java.util.zip}/{@code java.util.jar} - which can read files - or {@code java.nio.file}) would be
280+
* silently exposed. Listing each package explicitly keeps the perimeter closed: only the packages below are
281+
* visible, nothing else.
278282
* </p>
283+
* <p>Allowed packages (each member is visible unless explicitly denied):</p>
279284
* <ul>
280-
* <li>java.nio.*</li>
281-
* <li>java.lang.*</li>
282-
* <li>java.math.*</li>
283-
* <li>java.text.*</li>
284-
* <li>java.util.*</li>
285-
* <li>org.w3c.dom.*</li>
286-
* <li>org.apache.commons.jexl3.*</li>
287-
*
288-
* <li>org.apache.commons.jexl3 { JexlBuilder {} }</li>
289-
* <li>org.apache.commons.jexl3.introspection { JexlPermissions {} JexlPermissions$ClassPermissions {} }</li>
290-
* <li>org.apache.commons.jexl3.internal { Engine {} Engine32 {} TemplateEngine {} }</li>
291-
* <li>org.apache.commons.jexl3.internal.introspection { Uberspect {} Introspector {} }</li>
292-
* <li>java.lang { Runtime {} System {} ProcessBuilder {} Process {} RuntimePermission {} SecurityManager {} Thread {} ThreadGroup {} Class {} }</li>
293-
* <li>java.io { +PrintWriter {} +Writer {} +StringWriter {} +Reader {} +InputStream {} +OutputStream {} }</li>
294-
* <li>java.nio +{}</li>
295-
* <li>java.rmi {}</li>
285+
* <li>java.math</li>
286+
* <li>java.text</li>
287+
* <li>java.time, java.time.chrono, java.time.format, java.time.temporal, java.time.zone</li>
288+
* <li>java.util, java.util.concurrent, java.util.concurrent.atomic, java.util.function, java.util.stream, java.util.regex</li>
289+
* <li>java.nio, java.nio.charset</li>
290+
* <li>org.w3c.dom</li>
291+
* <li>java.lang (minus the denied classes below)</li>
292+
* <li>org.apache.commons.jexl3 (minus JexlBuilder)</li>
293+
* </ul>
294+
* <p>Denied classes / members (carved out of otherwise-allowed packages):</p>
295+
* <ul>
296+
* <li>java.lang { Runtime, System, ProcessBuilder, Process, RuntimePermission, SecurityManager, Thread, ThreadGroup, Class, ClassLoader }</li>
297+
* <li>java.io { everything except PrintWriter, Writer, StringWriter, Reader, InputStream, OutputStream }</li>
298+
* <li>java.util.concurrent { Executors and the thread-pool / fork-join executor classes }</li>
299+
* <li>java.time.zone { ZoneRulesProvider } (prevents JVM-wide time-zone provider registration)</li>
300+
* <li>org.apache.commons.jexl3 { JexlBuilder }</li>
296301
* </ul>
302+
* <p>Notably absent (and therefore denied) are file/IO/persistence/loader-bearing packages such as
303+
* {@code java.util.zip}, {@code java.util.jar}, {@code java.util.prefs}, {@code java.util.logging},
304+
* {@code java.util.concurrent.locks}, {@code java.nio.file}, {@code java.lang.reflect},
305+
* {@code java.lang.invoke} and {@code org.w3c.dom.ls}.</p>
297306
*/
298307

299308
JexlPermissions RESTRICTED = JexlPermissions.parse(
300309
"# Default Uberspect Permissions",
301-
"java.math.*",
302-
"java.text.*",
303-
"java.time.*",
304-
"java.util.*",
305-
"org.w3c.dom.*",
310+
"java.math +{}",
311+
"java.text +{}",
312+
"java.time +{}",
313+
"java.time.chrono +{}",
314+
"java.time.format +{}",
315+
"java.time.temporal +{}",
316+
"java.time.zone +{ -ZoneRulesProvider{} }",
317+
"java.util +{}",
318+
"java.util.concurrent +{" +
319+
"-Executors{} -ExecutorService{} -AbstractExecutorService{}" +
320+
"-ThreadPoolExecutor{} -ScheduledThreadPoolExecutor{} -ScheduledExecutorService{}" +
321+
"-ForkJoinPool{} -ForkJoinTask{} -ForkJoinWorkerThread{}" +
322+
"}",
323+
"java.util.concurrent.atomic +{}",
324+
"java.util.function +{}",
325+
"java.util.stream +{}",
326+
"java.util.regex +{}",
327+
"org.w3c.dom +{}",
306328
"java.lang +{" +
307329
"-Runtime{} -System{} -ProcessBuilder{} -Process{}" +
308330
"-RuntimePermission{} -SecurityManager{}" +

0 commit comments

Comments
 (0)