DEV Community

Cover image for Security Puzzle (Log4J Edition)
Philippe Arteau
Philippe Arteau

Posted on • Edited on • Originally published at gosecure.net

3 3

Security Puzzle (Log4J Edition)

Apache Log4J has been in the spotlight for its JNDI feature introducing a major security flaw in december.

Here is the function for JNDI lookups modified to avoid unexpected remote class being loaded. This function was used to fix potential RCE in version 2.15.

Can you see why the validation was insufficient?

private final DirContext context; //javax.naming.directory.DirContext

[...]

@SuppressWarnings("unchecked")
public <T> T lookup(final String name) throws NamingException {
public synchronized <T> T lookup(final String name) throws NamingException {

try {
    URI uri = new URI(name);
    if (uri.getScheme() != null) {
        if (!allowedProtocols.contains(uri.getScheme().toLowerCase(Locale.ROOT))) {
            LOGGER.warn("Log4j JNDI does not allow protocol {}", uri.getScheme());
            return null;
        }
        if (LDAP.equalsIgnoreCase(uri.getScheme()) || LDAPS.equalsIgnoreCase(uri.getScheme())) {
            if (!allowedHosts.contains(uri.getHost())) {
                LOGGER.warn("Attempt to access ldap server not in allowed list");
                return null;
            }
            Attributes attributes = this.context.getAttributes(name);
            if (attributes != null) {
                // In testing the "key" for attributes seems to be lowercase while the attribute id is
                // camelcase, but that may just be true for the test LDAP used here. This copies the Attributes
                // to a Map ignoring the "key" and using the Attribute's id as the key in the Map so it matches
                // the Java schema.
                Map<String, Attribute> attributeMap = new HashMap<>();
                NamingEnumeration<? extends Attribute> enumeration = attributes.getAll();
                while (enumeration.hasMore()) {
                    Attribute attribute = enumeration.next();
                    attributeMap.put(attribute.getID(), attribute);
                }
                Attribute classNameAttr = attributeMap.get(CLASS_NAME);
                if (attributeMap.get(SERIALIZED_DATA) != null) {
                    if (classNameAttr != null) {
                        String className = classNameAttr.get().toString();
                        if (!allowedClasses.contains(className)) {
                            LOGGER.warn("Deserialization of {} is not allowed", className);
                            return null;
                        }
                    } else {
                        LOGGER.warn("No class name provided for {}", name);
                        return null;
                    }
                } else if (attributeMap.get(REFERENCE_ADDRESS) != null
                        || attributeMap.get(OBJECT_FACTORY) != null) {
                    LOGGER.warn("Referenceable class is not allowed for {}", name);
                    return null;
                }
            }
        }
    }
} catch (URISyntaxException ex) {
    // This is OK.
}
return (T) this.context.lookup(name);

}
Enter fullscreen mode Exit fullscreen mode

Ref: Changeset

What is/are the weakness in the code above?

  • A) Incomplete blacklist / whitelist
  • B) Improper URL parsing
  • C) Unexpected Nullpointer
  • D) Exception handling
  • E) Time of Check, Time of Use

Answer (SPOILER!)

It is both a TOCTOU and a URL parsing issue!

Did you identify either weakness?




Billboard image

The fastest way to detect downtimes

Join Vercel, CrowdStrike, and thousands of other teams that trust Checkly to streamline monitoring.

Get started now

Top comments (0)

Sentry image

See why 4M developers consider Sentry, “not bad.”

Fixing code doesn’t have to be the worst part of your day. Learn how Sentry can help.

Learn more

👋 Kindness is contagious

Explore a sea of insights with this enlightening post, highly esteemed within the nurturing DEV Community. Coders of all stripes are invited to participate and contribute to our shared knowledge.

Expressing gratitude with a simple "thank you" can make a big impact. Leave your thanks in the comments!

On DEV, exchanging ideas smooths our way and strengthens our community bonds. Found this useful? A quick note of thanks to the author can mean a lot.

Okay