Security Puzzle (Log4J Edition)
Tags: security, java, challenge, log4j
Published: 2022-01-10 16:16:00

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

Here is the function for <abbr title="Java Naming and Directory Interface">JNDI</abbr> lookups modified to avoid unexpected remote class being loaded. This function was used to fix potential <abbr title="Remote Code Execution">RCE</abbr> in version 2.15.

Can you see why the validation was **insufficient**?

```java
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);
	
}
```
Ref: [Changeset](https://github.com/apache/logging-log4j2/commit/d82b47c6fae9c15fcb183170394d5f1a01ac02d3#diff-271353c1076e53f6893261e4420de27d34588bfd782806b5c66a3465c43b7f51R209)


## 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

{% spoiler Answer (SPOILER!)%}
> It is both a **TOCTOU** and a **URL parsing** issue!
> - TOCTOU explanation: https://www.gosecure.net/blog/2021/12/21/log4j-2-15-toctou-vulnerability-illustrated-by-gosecure-researchers/
> - DNS re-parsing issue: https://www.lunasec.io/docs/blog/log4j-zero-day-severity-of-cve-2021-45046-increased/
>  
> Did you identify either weakness?
{% endspoiler %}