Copilot commented on code in PR #501:
URL: https://github.com/apache/commons-bcel/pull/501#discussion_r3407976821
##########
src/test/java/org/apache/bcel/util/BCELifierTest.java:
##########
@@ -279,6 +288,30 @@ void testStackMap(final String className) throws Exception
{
assertEquals("Hello World" + EOL, exec(workDir, getAppJava(), "-cp",
CLASSPATH, className, "Hello"));
}
+ @Test
+ void testCreateInvokeEscapesConstantPoolName() throws Exception {
+ // A hostile constant pool can hold any UTF-8 as a referenced method
name.
+ final String evilName = "evil\"); System.exit(1); il.append(\"";
+ final ClassGen cg = new ClassGen("Example", "java.lang.Object",
"Example.java", Const.ACC_PUBLIC | Const.ACC_SUPER, new String[] {});
+ final ConstantPoolGen cp = cg.getConstantPool();
+ final InstructionFactory factory = new InstructionFactory(cg, cp);
+ final InstructionList il = new InstructionList();
+ il.append(InstructionConst.ALOAD_0);
+ il.append(factory.createInvoke("java.lang.Object", evilName,
Type.VOID, Type.NO_ARGS, Const.INVOKEVIRTUAL));
+ il.append(InstructionConst.RETURN);
+ final MethodGen mg = new MethodGen(Const.ACC_PUBLIC, Type.VOID,
Type.NO_ARGS, new String[] {}, "m", "Example", il, cp);
+ mg.setMaxStack();
+ mg.setMaxLocals();
+ cg.addMethod(mg.getMethod());
+
Review Comment:
This covers escaping for method names referenced by invoke instructions, but
BCELifier still emits several other constant-pool-derived strings into Java
string literals without escaping (e.g., class name/superclass/source file in
the `new ClassGen(...)` line, interface names via `Utility.printArray(...,
quote=true)`, and `method.getName()` in the `new MethodGen(...)` line). A
crafted class name/interface/method name containing `"` can still break out of
the generated string literal and inject code even with this change in
BCELFactory. Consider extending the escaping to those emitters as well and
adding tests that exercise at least class name and method declaration name
escaping.
##########
src/test/java/org/apache/bcel/util/BCELifierTest.java:
##########
@@ -279,6 +288,30 @@ void testStackMap(final String className) throws Exception
{
assertEquals("Hello World" + EOL, exec(workDir, getAppJava(), "-cp",
CLASSPATH, className, "Hello"));
}
+ @Test
+ void testCreateInvokeEscapesConstantPoolName() throws Exception {
+ // A hostile constant pool can hold any UTF-8 as a referenced method
name.
+ final String evilName = "evil\"); System.exit(1); il.append(\"";
+ final ClassGen cg = new ClassGen("Example", "java.lang.Object",
"Example.java", Const.ACC_PUBLIC | Const.ACC_SUPER, new String[] {});
+ final ConstantPoolGen cp = cg.getConstantPool();
+ final InstructionFactory factory = new InstructionFactory(cg, cp);
+ final InstructionList il = new InstructionList();
+ il.append(InstructionConst.ALOAD_0);
+ il.append(factory.createInvoke("java.lang.Object", evilName,
Type.VOID, Type.NO_ARGS, Const.INVOKEVIRTUAL));
+ il.append(InstructionConst.RETURN);
+ final MethodGen mg = new MethodGen(Const.ACC_PUBLIC, Type.VOID,
Type.NO_ARGS, new String[] {}, "m", "Example", il, cp);
+ mg.setMaxStack();
+ mg.setMaxLocals();
+ cg.addMethod(mg.getMethod());
+
+ final ByteArrayOutputStream os = new ByteArrayOutputStream();
+ new BCELifier(cg.getJavaClass(), os).start();
+ final String source = new String(os.toByteArray(),
StandardCharsets.UTF_8);
+
+ assertTrue(source.contains(Utility.convertString(evilName)), source);
+ assertFalse(source.contains('"' + evilName + '"'), source);
Review Comment:
This test’s positive assertion is fairly loose: it only checks that the
escaped substring appears somewhere in the generated source. Tightening it to
assert the escaped name appears specifically as the quoted `createInvoke`
argument would reduce the chance of false positives if the output format
changes or the escaped substring appears in another context.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
