CVE-2022-24897 (Java, published 2022-05-02T22:15Z)
Description: value: 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.
Severity: MEDIUM (CVSS v3 base score: 7.5)
Weakness: CWE-22 \u2014 Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
Repository: https://github.com/xwiki/xwiki-commons
Files changed:
xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/SecureIntrospector.java
xwiki-commons-core/xwiki-commons-velocity/src/test/java/org/xwiki/velocity/introspection/SecureIntrospectorTest.java
Commit message: XWIKI-5168: Don't allow some methods in velocity introspector (#127)



Vulnerable code:
    private final Set<String> secureClassMethods = new HashSet<>();
        this.secureClassMethods.add("getname");
        this.secureClassMethods.add("getName");
        this.secureClassMethods.add("getsimpleName");
        this.secureClassMethods.add("getSimpleName");
        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");
        // TODO: add more when needed
        if (Class.class.isAssignableFrom(clazz) && methodName != null && this.secureClassMethods.contains(methodName)) {
            return true;
        } else {
            return super.checkObjectExecutePermission(clazz, methodName);

Fixed code:
import java.io.File;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
    private static final String GETNAME = "getname";
    private final Map<Class, Set<String>> whitelistedMethods;
        this.whitelistedMethods = new HashMap<>();
        this.prepareWhitelistClass();
        this.prepareWhiteListFile();
    }
    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);
    }
    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);
        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;
/*
 * See the NOTICE file distributed with this work for additional
 * information regarding copyright ownership.
 *
 * This is free software; you can redistribute it and/or modify it
 * under the terms of the GNU Lesser General Public License as
 * published by the Free Software Foundation; either version 2.1 of
 * the License, or (at your option) any later version.
 *
 * This software is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
 * Lesser General Public License for more details.
 *
 * You should have received a copy of the GNU Lesser General Public
 * License along with this software; if not, write to the Free
 * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
 * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
 */
package org.xwiki.velocity.introspection;

import java.io.File;
import java.util.ArrayList;

import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.slf4j.Logger;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
 * Tests for {@link SecureIntrospector}.
 *
 * @version $Id$
 */
class SecureIntrospectorTest
{
    @Mock
    private Logger logger;

    class CustomFile extends File
    {
        public CustomFile(String s)
        {
            super(s);
        }
    }

    @Test
    void checkObjectExecutePermissionWithClass()
    {
        SecureIntrospector secureIntrospector = new SecureIntrospector(new String[] {}, new String[] {}, this.logger);
        assertTrue(secureIntrospector.checkObjectExecutePermission(Class.class, "isLocalClass"));
    }

    @Test
    void checkObjectExecutePermissionWithFile()
    {
        SecureIntrospector secureIntrospector = new SecureIntrospector(new String[] {}, new String[] {}, this.logger);
        assertTrue(secureIntrospector.checkObjectExecutePermission(File.class, "toString"));
        assertFalse(secureIntrospector.checkObjectExecutePermission(File.class, "mkdir"));

        assertTrue(secureIntrospector.checkObjectExecutePermission(File.class, "tostring"));
        assertFalse(secureIntrospector.checkObjectExecutePermission(File.class, "renameto"));
        assertFalse(secureIntrospector.checkObjectExecutePermission(File.class, "renameTo"));

        assertTrue(secureIntrospector.checkObjectExecutePermission(CustomFile.class, "toString"));
        assertFalse(secureIntrospector.checkObjectExecutePermission(CustomFile.class, "mkdir"));
    }

    @Test
    void checkObjectExecutePermissionBlacklistedClass()
    {
        SecureIntrospector secureIntrospector = new SecureIntrospector(
            new String[] { "java.util.ArrayList" }, new String[] {}, this.logger);
        assertTrue(secureIntrospector.checkObjectExecutePermission(File.class, "toString"));
        assertFalse(secureIntrospector.checkObjectExecutePermission(ArrayList.class, "toString"));
    }
}