Headline
CVE-2022-24897: XWIKI-5168: Don't allow some methods in velocity introspector (#127) · xwiki/xwiki-commons@215951c
APIs to evaluate content with Velocity is a package for APIs to evaluate content with Velocity. Starting with version 2.3 and prior to 12.6.7, 12.10.3, and 13.0, the velocity scripts are not properly sandboxed against using the Java File API to perform read or write operations on the filesystem. Writing an attacking script in Velocity requires the Script rights in XWiki so not all users can use it, and it also requires finding an XWiki API which returns a File. The problem has been patched in versions 12.6.7, 12.10.3, and 13.0. There is no easy workaround for fixing this vulnerability other than upgrading and being careful when giving Script rights.
@@ -19,7 +19,11 @@ */ package org.xwiki.velocity.introspection;
import java.io.File; import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set;
import org.apache.velocity.util.introspection.SecureIntrospectorImpl; @@ -33,7 +37,8 @@ */ public class SecureIntrospector extends SecureIntrospectorImpl { private final Set<String> secureClassMethods = new HashSet<>(); private static final String GETNAME = "getname"; private final Map<Class, Set<String>> whitelistedMethods;
/** * @param badClasses forbidden classes @@ -44,41 +49,82 @@ public SecureIntrospector(String[] badClasses, String[] badPackages, Logger log) { super(badClasses, badPackages, log);
this.secureClassMethods.add(“getname”); this.secureClassMethods.add(“getName”); this.secureClassMethods.add(“getsimpleName”); this.secureClassMethods.add(“getSimpleName”); this.whitelistedMethods = new HashMap<>(); this.prepareWhitelistClass(); this.prepareWhiteListFile(); }
this.secureClassMethods.add(“isarray”); this.secureClassMethods.add(“isArray”); this.secureClassMethods.add(“isassignablefrom”); this.secureClassMethods.add(“isAssignableFrom”); this.secureClassMethods.add(“isenum”); this.secureClassMethods.add(“isEnum”); this.secureClassMethods.add(“isinstance”); this.secureClassMethods.add(“isInstance”); this.secureClassMethods.add(“isinterface”); this.secureClassMethods.add(“isInterface”); this.secureClassMethods.add(“islocalClass”); this.secureClassMethods.add(“isLocalClass”); this.secureClassMethods.add(“ismemberclass”); this.secureClassMethods.add(“isMemberClass”); this.secureClassMethods.add(“isprimitive”); this.secureClassMethods.add(“isPrimitive”); this.secureClassMethods.add(“issynthetic”); this.secureClassMethods.add(“isSynthetic”); this.secureClassMethods.add(“getEnumConstants”); private void prepareWhitelistClass() { Set<String> whitelist = new HashSet<>(Arrays.asList( GETNAME, "getsimpleName", "isarray", "isassignablefrom", "isenum", "isinstance", "isinterface", "islocalclass", "ismemberclass", "isprimitive", "issynthetic", “getenumconstants” )); this.whitelistedMethods.put(Class.class, whitelist); }
// TODO: add more when needed private void prepareWhiteListFile() { Set<String> whitelist = new HashSet<>(Arrays.asList( "canexecute", "canread", "canwrite", "compareto", "createtempfile", "equals", "getabsolutefile", "getabsolutepath", "getcanonicalfile", "getcanonicalpath", "getfreespace", GETNAME, "getparent", "getparentfile", "getpath", "gettotalspace", "getusablespace", "hashcode", "isabsolute", "isdirectory", "isfile", "ishidden", "lastmodified", "length", "topath", "tostring", "touri", "tourl", “getclass” )); this.whitelistedMethods.put(File.class, whitelist); }
@Override public boolean checkObjectExecutePermission(Class clazz, String methodName) { if (Class.class.isAssignableFrom(clazz) && methodName != null && this.secureClassMethods.contains(methodName)) { return true; } else { return super.checkObjectExecutePermission(clazz, methodName); Boolean result = null; if (methodName != null) { for (Map.Entry<Class, Set<String>> classSetEntry : this.whitelistedMethods.entrySet()) { if (classSetEntry.getKey().isAssignableFrom(clazz)) { result = classSetEntry.getValue().contains(methodName.toLowerCase()); break; } } }
if (result == null) { result = super.checkObjectExecutePermission(clazz, methodName); } return result; } }