-
Notifications
You must be signed in to change notification settings - Fork 71
Log4j Disable Literal Pattern Converter #46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: decoupled
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,6 +46,19 @@ function start_static_target() { | |
popd > /dev/null | ||
} | ||
|
||
function start_dos_test() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a test that verifies by default the dos patcher is not active? |
||
if [[ -f /tmp/vuln2.log ]]; then | ||
rm /tmp/vuln2.log | ||
fi | ||
local jdk_dir=$1 | ||
local agent_jar=$2 | ||
|
||
pushd "${ROOT_DIR}/test" > /dev/null | ||
${jdk_dir}/bin/java -cp log4j-core-2.12.1.jar:log4j-api-2.12.1.jar:. -Dlog4j2.configurationFile=${ROOT_DIR}/src/test/resources/log4j-vuln2.properties -javaagent:${agent_jar}=patcherClassName=com.amazon.corretto.hotpatch.patch.impl.set.Log4j2PatchSetWithDisableLookups Vuln2 > /tmp/vuln2.log & | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you hotpatch with this? If so please add tests and doc updates |
||
|
||
popd > /dev/null | ||
} | ||
|
||
function static_agent_configure_verbose() { | ||
if [ "$3" = "unset" ]; then | ||
PROP_VALUE="" | ||
|
@@ -107,6 +120,25 @@ function verify_idempotent_agent() { | |
fi | ||
} | ||
|
||
function verify_dos_test() { | ||
if grep -q '${ctx:myvar} - Any string' /tmp/vuln2.log | ||
then | ||
echo "Test passed. JVM did not run into StackOverflow" | ||
else | ||
echo "Test failed. JVM failed." | ||
cat /tmp/vuln2.log | ||
exit 1 | ||
fi | ||
if grep -q '${ctx:myvar} - Patched JndiLookup::lookup()' /tmp/vuln2.log | ||
then | ||
echo "Test passed. Patched JndiLookup::lookup" | ||
else | ||
echo "Test failed. Did not patch JndiLookup" | ||
cat /tmp/vuln2.log | ||
exit 1 | ||
fi | ||
} | ||
|
||
if [[ $# -lt 2 ]]; then | ||
usage | ||
exit 1 | ||
|
@@ -165,7 +197,7 @@ if [[ "${JVM_MV}" == "17" ]]; then | |
fi | ||
|
||
pushd "${ROOT_DIR}/test" > /dev/null | ||
${JDK_DIR}/bin/javac -cp log4j-core-2.12.1.jar:log4j-api-2.12.1.jar Vuln.java Empty.java | ||
${JDK_DIR}/bin/javac -cp log4j-core-2.12.1.jar:log4j-api-2.12.1.jar Vuln.java Empty.java Vuln2.java | ||
popd > /dev/null | ||
|
||
echo | ||
|
@@ -353,7 +385,17 @@ if [[ -z "${SKIP_STATIC}" ]]; then | |
start_target ${JDK_DIR} | ||
VULN_PID=$! | ||
|
||
|
||
sleep 2 | ||
|
||
verify_target $VULN_PID | ||
unset _JAVA_OPTIONS | ||
echo | ||
echo "******************" | ||
echo "Running Literal Pattern Converter JDK${JVM_MV} Test" | ||
echo "------------------" | ||
|
||
start_dos_test ${JDK_DIR} ${AGENT_JAR} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need to capture the pid of the process and kill it later. |
||
sleep 2 | ||
verify_dos_test | ||
fi |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,185 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"). | ||
* You may not use this file except in compliance with the License. | ||
* A copy of the License is located at | ||
* | ||
* http://aws.amazon.com/apache2.0 | ||
* | ||
* or in the "license" file accompanying this file. This file is distributed | ||
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either | ||
* express or implied. See the License for the specific language governing | ||
* permissions and limitations under the License. | ||
*/ | ||
|
||
package com.amazon.corretto.hotpatch.patch.impl.log4j2; | ||
|
||
import com.amazon.corretto.hotpatch.org.objectweb.asm.ClassReader; | ||
import com.amazon.corretto.hotpatch.org.objectweb.asm.ClassVisitor; | ||
import com.amazon.corretto.hotpatch.org.objectweb.asm.ClassWriter; | ||
import com.amazon.corretto.hotpatch.org.objectweb.asm.Label; | ||
import com.amazon.corretto.hotpatch.org.objectweb.asm.MethodVisitor; | ||
import com.amazon.corretto.hotpatch.org.objectweb.asm.Opcodes; | ||
import com.amazon.corretto.hotpatch.patch.ClassTransformerHotPatch; | ||
|
||
public class Log4j2DisableLiteralPatternConverter implements ClassTransformerHotPatch { | ||
static final String CLASS_NAME = "org.apache.logging.log4j.core.pattern.LiteralPatternConverter"; | ||
static final String CLASS_NAME_SLASH = CLASS_NAME.replace(".", "/"); | ||
|
||
private final static String NAME = "Log4j2_DisableLiteralPatternConverter"; | ||
|
||
@Override | ||
public String getName() { | ||
return NAME; | ||
} | ||
|
||
@Override | ||
public String getDescription() { | ||
return "Fixes CVE-2021-45105 by patching the LiteralPatternConverter to disable lookup in message patterns."; | ||
} | ||
|
||
@Override | ||
public boolean isTargetClass(String className) { | ||
return className.endsWith(CLASS_NAME) || className.endsWith(CLASS_NAME_SLASH); | ||
} | ||
|
||
public static boolean isEnabled(String args) { | ||
String param = "--enable-" + NAME; | ||
return args != null && args.contains(param); | ||
} | ||
Comment on lines
+47
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think isEnabled is in the current implementation |
||
|
||
@Override | ||
public byte[] apply(int asmApiVersion, String className, byte[] classfileBuffer) { | ||
ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); | ||
ClassVisitor cv = new DisableLiteralPatternConverterClassVisitor(asmApiVersion, cw); | ||
ClassReader cr = new ClassReader(classfileBuffer); | ||
cr.accept(cv, 0); | ||
return cw.toByteArray(); | ||
} | ||
|
||
public static class DisableLiteralPatternConverterClassVisitor extends ClassVisitor { | ||
|
||
public DisableLiteralPatternConverterClassVisitor(int asmApiVersion, ClassVisitor classVisitor) { | ||
super(asmApiVersion, classVisitor); | ||
} | ||
|
||
@Override | ||
public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) { | ||
MethodVisitor mv = cv.visitMethod(access, name, desc, signature, exceptions); | ||
if ("format".equals(name)) { | ||
mv = new LiteralPatternConverterMethodVisitor(api, mv); | ||
} | ||
return mv; | ||
} | ||
} | ||
|
||
public static class LiteralPatternConverterMethodVisitor extends MethodVisitor implements Opcodes { | ||
private static final String OWNER = CLASS_NAME_SLASH; | ||
private static final String DESC = "Z"; | ||
private static final String NAME = "substitute"; | ||
enum State { | ||
CLEAR, | ||
LOADED_SUBSTITUTE, | ||
} | ||
|
||
private State state = State.CLEAR; | ||
|
||
public LiteralPatternConverterMethodVisitor(int asmApiVersion, MethodVisitor methodVisitor) { | ||
super(asmApiVersion, methodVisitor); | ||
} | ||
|
||
@Override | ||
public void visitFieldInsn(int opc, String owner, String name, String desc) { | ||
if (OWNER.equals(owner) && NAME.equals(name) && DESC.equals(desc) && opc == GETFIELD) { | ||
visitState(); | ||
} else { | ||
clearState(); | ||
} | ||
mv.visitFieldInsn(opc, owner, name, desc); | ||
} | ||
|
||
@Override | ||
public void visitJumpInsn(int opc, Label label) { | ||
mv.visitJumpInsn(opc, label); | ||
if (state == State.LOADED_SUBSTITUTE && opc == IFEQ) { | ||
mv.visitJumpInsn(GOTO, label); | ||
} | ||
clearState(); | ||
|
||
} | ||
|
||
private void clearState() { | ||
state = State.CLEAR; | ||
} | ||
|
||
private void visitState() { | ||
state = State.LOADED_SUBSTITUTE; | ||
} | ||
|
||
@Override | ||
public void visitVarInsn(int opcode, int var) { | ||
clearState(); | ||
mv.visitVarInsn(opcode, var); | ||
} | ||
|
||
@Override | ||
public void visitTypeInsn(int opcode, String desc) { | ||
clearState(); | ||
mv.visitTypeInsn(opcode, desc); | ||
} | ||
|
||
@Override | ||
public void visitMethodInsn(int opcode, String owner, String name, String desc) { | ||
clearState(); | ||
mv.visitMethodInsn(opcode, owner, name, desc); | ||
} | ||
|
||
@Override | ||
public void visitLabel(Label label) { | ||
mv.visitLabel(label); | ||
} | ||
|
||
@Override | ||
public void visitLdcInsn(Object cst) { | ||
clearState(); | ||
mv.visitLdcInsn(cst); | ||
} | ||
|
||
@Override | ||
public void visitIincInsn(int var, int increment) { | ||
clearState(); | ||
mv.visitIincInsn(var, increment); | ||
} | ||
|
||
@Override | ||
public void visitTableSwitchInsn(int min, int max, Label dflt, Label[] labels) { | ||
mv.visitTableSwitchInsn(min, max, dflt, labels); | ||
} | ||
|
||
@Override | ||
public void visitLookupSwitchInsn(Label dflt, int[] keys, Label[] labels) { | ||
mv.visitLookupSwitchInsn(dflt, keys, labels); | ||
} | ||
|
||
@Override | ||
public void visitMultiANewArrayInsn(String desc, int dims) { | ||
mv.visitMultiANewArrayInsn(desc, dims); | ||
} | ||
|
||
@Override | ||
public void visitTryCatchBlock(Label start, Label end, Label handler, String type) { | ||
mv.visitTryCatchBlock(start, end, handler, type); | ||
} | ||
|
||
@Override | ||
public void visitLocalVariable(String name, String desc, String signature, Label start, Label end, int index) { | ||
mv.visitLocalVariable(name, desc, signature, start, end, index); | ||
} | ||
|
||
@Override | ||
public void visitLineNumber(int line, Label start) { | ||
mv.visitLineNumber(line, start); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"). | ||
* You may not use this file except in compliance with the License. | ||
* A copy of the License is located at | ||
* | ||
* http://aws.amazon.com/apache2.0 | ||
* | ||
* or in the "license" file accompanying this file. This file is distributed | ||
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either | ||
* express or implied. See the License for the specific language governing | ||
* permissions and limitations under the License. | ||
*/ | ||
|
||
package com.amazon.corretto.hotpatch.patch.impl.set; | ||
|
||
import java.util.Arrays; | ||
import java.util.Collections; | ||
import java.util.List; | ||
|
||
import com.amazon.corretto.hotpatch.patch.ClassTransformerHotPatch; | ||
import com.amazon.corretto.hotpatch.patch.impl.log4j2.Log4j2DisableLiteralPatternConverter; | ||
import com.amazon.corretto.hotpatch.patch.impl.log4j2.Log4j2NoJndiLookup; | ||
|
||
/** | ||
* Patch set contains the following patches | ||
* {@link Log4j2NoJndiLookup} | ||
* {@link Log4j2DisableLiteralPatternConverter} | ||
*/ | ||
public class Log4j2PatchSetWithDisableLookups extends PatchSetPatcher { | ||
private final List<ClassTransformerHotPatch> patches = Collections.unmodifiableList(Arrays.asList( | ||
(ClassTransformerHotPatch) new Log4j2NoJndiLookup(), | ||
(ClassTransformerHotPatch) new Log4j2DisableLiteralPatternConverter() | ||
)); | ||
|
||
@Override | ||
public List<ClassTransformerHotPatch> getPatches() { | ||
return patches; | ||
} | ||
|
||
@Override | ||
public String getName() { | ||
return "log4j2"; | ||
} | ||
|
||
@Override | ||
public int getVersion() { | ||
return 2; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the version for the patchset, if so this is version 1 of Log4j2PatchSetWithDisableLookups. |
||
} | ||
|
||
@Override | ||
public String getShortDescription() { | ||
return "Fix vulnerabilities in Log4j2 related to message lookups and recursive lookups"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
appender.console.type = Console | ||
appender.console.name = STDOUT | ||
appender.console.layout.type = PatternLayout | ||
appender.console.layout.pattern = ${ctx:myvar} - %m%n | ||
|
||
rootLogger.level = info | ||
rootLogger.appenderRef.stdout.ref = STDOUT |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
import org.apache.logging.log4j.ThreadContext; | ||
|
||
public class Vuln2 { | ||
private static final Logger logger = LogManager.getLogger(Vuln2.class); | ||
public static void main(String[] args) throws Exception { | ||
// Have an appender that reads myvar configured like | ||
// appender.console.layout.pattern = ${ctx:myvar} - %m%n | ||
ThreadContext.put("myvar", "${${ctx:myvar}}"); | ||
logger.error("Any string"); | ||
logger.error("${jndi:ldap://localhost:4444/exp}"); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please expand on when/why some one would need to patch for this and what the side effects of it are.