This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-bcel.git
commit fb72c225cbc6ec3d94060ed6edb269f07428d504 Author: Gary Gregory <[email protected]> AuthorDate: Fri Sep 4 17:19:41 2026 -0400 Class2HTML emitters write attacker class-file strings into HTML unescaped (stored XSS in reports) (f014). --- src/changes/changes.xml | 1 + .../java/org/apache/bcel/util/AttributeHTML.java | 4 ++-- src/main/java/org/apache/bcel/util/Class2HTML.java | 20 +++++++++++++++++++- src/main/java/org/apache/bcel/util/CodeHTML.java | 2 +- .../java/org/apache/bcel/util/ConstantHTML.java | 8 ++++---- src/main/java/org/apache/bcel/util/MethodHTML.java | 7 ++++--- .../org/apache/bcel/util/Class2HTMLXSSTest.java | 21 +++++++++++++++++++++ 7 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 662e9ea9..09084eb2 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -101,6 +101,7 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Gary Gregory">Utility.typeSignatureToString recurses per generic-nesting level, unbounded, with quadratic substring copies. (f011).</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Repositories cache classes under the input-defined this_class name; global static Repository/VerifierFactory make the poisoning ClassLoader-wide. (f012).</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Silent u2/u1 count truncation across dump paths corrupts emitted bytecode. (f013).</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">Class2HTML emitters write attacker class-file strings into HTML unescaped (stored XSS in reports) (f014).</action> <!-- ADD --> <action type="add" dev="ggregory" due-to="nbauma109, Gary Gregory">Add support for permitted subclasses #493.</action> <action type="add" dev="ggregory" due-to="nbauma109, Gary Gregory">Add RecordComponentInfo.getAttribute(byte tag)#494.</action> diff --git a/src/main/java/org/apache/bcel/util/AttributeHTML.java b/src/main/java/org/apache/bcel/util/AttributeHTML.java index f87b1b24..950d74fd 100644 --- a/src/main/java/org/apache/bcel/util/AttributeHTML.java +++ b/src/main/java/org/apache/bcel/util/AttributeHTML.java @@ -88,7 +88,7 @@ final class AttributeHTML implements Closeable { } else { printWriter.print("<TR BGCOLOR=\"#A0A0A0\"><TD>"); } - printWriter.println("<H4><A NAME=\"" + anchor + "\">" + attrCount + " " + Const.getAttributeName(tag) + "</A></H4>"); + printWriter.println("<H4><A NAME=\"" + Class2HTML.toHTML(anchor) + "\">" + attrCount + " " + Const.getAttributeName(tag) + "</A></H4>"); /* * Handle different attributes */ @@ -184,7 +184,7 @@ final class AttributeHTML implements Closeable { printWriter.print("</UL>\n"); break; default: // Such as Unknown attribute or Deprecated - printWriter.print("<P>" + attribute); + printWriter.print("<P>" + Class2HTML.toHTML(String.valueOf(attribute))); } printWriter.println("</TD></TR>"); printWriter.flush(); diff --git a/src/main/java/org/apache/bcel/util/Class2HTML.java b/src/main/java/org/apache/bcel/util/Class2HTML.java index f103360b..2047f7f2 100644 --- a/src/main/java/org/apache/bcel/util/Class2HTML.java +++ b/src/main/java/org/apache/bcel/util/Class2HTML.java @@ -150,7 +150,16 @@ public class Class2HTML implements Constants { if (basicTypes.contains(baseType)) { return "<FONT COLOR=\"#00FF00\">" + type + "</FONT>"; } - return "<A HREF=\"" + baseType + ".html\" TARGET=_top>" + toHTML(shortType) + "</A>"; + return "<A HREF=\"" + toHTMLRef(baseType) + ".html\" TARGET=_top>" + toHTML(shortType) + "</A>"; + } + + /** + * Escapes a class or type name taken from the constant pool for use as a relative link target inside an HREF + * attribute value. On top of the text escaping done by {@code toHTML(String)}, any ':' is replaced so an + * attacker-chosen name cannot smuggle a URL scheme such as "javascript:" into the generated link. + */ + static String toHTMLRef(final String str) { + return toHTML(str.replace(':', '_')); } static String toHTML(final String str) { @@ -158,12 +167,21 @@ public class Class2HTML implements Constants { for (int i = 0; i < str.length(); i++) { final char ch; switch (ch = str.charAt(i)) { + case '&': + buf.append("&"); + break; case '<': buf.append("<"); break; case '>': buf.append(">"); break; + case '"': + buf.append("""); + break; + case '\'': + buf.append("'"); + break; case '\n': buf.append("\\n"); break; diff --git a/src/main/java/org/apache/bcel/util/CodeHTML.java b/src/main/java/org/apache/bcel/util/CodeHTML.java index 3b9d1b90..c23527a3 100644 --- a/src/main/java/org/apache/bcel/util/CodeHTML.java +++ b/src/main/java/org/apache/bcel/util/CodeHTML.java @@ -243,7 +243,7 @@ final class CodeHTML { index = c1.getNameAndTypeIndex(); final String fieldName = constantPool.constantToString(index, Const.CONSTANT_NameAndType); if (name.equals(className)) { // Local field - buf.append("<A HREF=\"").append(className).append("_methods.html#field").append(fieldName).append("\" TARGET=Methods>") + buf.append("<A HREF=\"").append(className).append("_methods.html#field").append(Class2HTML.toHTML(fieldName)).append("\" TARGET=Methods>") .append(Class2HTML.toHTML(fieldName)).append("</A>\n"); } else { buf.append(constantHtml.referenceConstant(classIndex)).append(".").append(Class2HTML.toHTML(fieldName)); diff --git a/src/main/java/org/apache/bcel/util/ConstantHTML.java b/src/main/java/org/apache/bcel/util/ConstantHTML.java index a85e1cd3..7f3da35e 100644 --- a/src/main/java/org/apache/bcel/util/ConstantHTML.java +++ b/src/main/java/org/apache/bcel/util/ConstantHTML.java @@ -144,7 +144,7 @@ final class ConstantHTML { if (methodClass.equals(className)) { ref = "<A HREF=\"" + className + "_code.html#method" + getMethodNumber(methodName + signature) + "\" TARGET=Code>" + htmlMethodName + "</A>"; } else { - ref = "<A HREF=\"" + methodClass + ".html\" TARGET=_top>" + shortMethodClass + "</A>." + htmlMethodName; + ref = "<A HREF=\"" + Class2HTML.toHTMLRef(methodClass) + ".html\" TARGET=_top>" + shortMethodClass + "</A>." + htmlMethodName; } constantRef[index] = retType + " <A HREF=\"" + className + "_cp.html#cp" + classIndex + "\" TARGET=Constants>" + shortMethodClass + "</A>.<A HREF=\"" + className + "_cp.html#cp" + index + "\" TARGET=ConstantPool>" + htmlMethodName + "</A> " + argTypes; @@ -164,9 +164,9 @@ final class ConstantHTML { final String fieldName = constantPool.constantToString(nameIndex, Const.CONSTANT_NameAndType); final String htmlFieldName = Class2HTML.toHTML(fieldName); if (fieldClass.equals(className)) { - ref = "<A HREF=\"" + fieldClass + "_methods.html#field" + fieldName + "\" TARGET=Methods>" + htmlFieldName + "</A>"; + ref = "<A HREF=\"" + Class2HTML.toHTMLRef(fieldClass) + "_methods.html#field" + htmlFieldName + "\" TARGET=Methods>" + htmlFieldName + "</A>"; } else { - ref = "<A HREF=\"" + fieldClass + ".html\" TARGET=_top>" + shortFieldClass + "</A>." + htmlFieldName + "\n"; + ref = "<A HREF=\"" + Class2HTML.toHTMLRef(fieldClass) + ".html\" TARGET=_top>" + shortFieldClass + "</A>." + htmlFieldName + "\n"; } constantRef[index] = "<A HREF=\"" + className + "_cp.html#cp" + classIndex + "\" TARGET=Constants>" + shortFieldClass + "</A>.<A HREF=\"" + className + "_cp.html#cp" + index + "\" TARGET=ConstantPool>" + htmlFieldName + "</A>"; @@ -180,7 +180,7 @@ final class ConstantHTML { String shortClassName = Utility.compactClassName(className2); // I.e., remove java.lang. shortClassName = Utility.compactClassName(shortClassName, classPackage + ".", true); // Remove class package prefix shortClassName = Class2HTML.toHTML(shortClassName); - ref = "<A HREF=\"" + className2 + ".html\" TARGET=_top>" + shortClassName + "</A>"; + ref = "<A HREF=\"" + Class2HTML.toHTMLRef(className2) + ".html\" TARGET=_top>" + shortClassName + "</A>"; constantRef[index] = "<A HREF=\"" + className + "_cp.html#cp" + index + "\" TARGET=ConstantPool>" + shortClassName + "</A>"; printWriter.println("<P><TT>" + ref + "</TT><UL>" + "<LI><A HREF=\"#cp" + nameIndex + "\">Name index(" + nameIndex + ")</A></UL>\n"); break; diff --git a/src/main/java/org/apache/bcel/util/MethodHTML.java b/src/main/java/org/apache/bcel/util/MethodHTML.java index d6c053de..8b43ea2d 100644 --- a/src/main/java/org/apache/bcel/util/MethodHTML.java +++ b/src/main/java/org/apache/bcel/util/MethodHTML.java @@ -78,7 +78,7 @@ final class MethodHTML { final Attribute[] attributes; access = Utility.replace(access, " ", " "); printWriter.print("<TR><TD><FONT COLOR=\"#FF0000\">" + access + "</FONT></TD>\n<TD>" + Class2HTML.referenceType(type) + "</TD><TD><A NAME=\"field" - + name + "\">" + Class2HTML.toHTML(name) + "</A></TD>"); + + Class2HTML.toHTML(name) + "\">" + Class2HTML.toHTML(name) + "</A></TD>"); attributes = field.getAttributes(); // Write them to the Attributes.html file with anchor "<name>[<i>]" for (int i = 0; i < attributes.length; i++) { @@ -88,7 +88,8 @@ final class MethodHTML { if (attributes[i].getTag() == Const.ATTR_CONSTANT_VALUE) { // Default value final String str = attributes[i].toString(); // Reference attribute in _attributes.html - printWriter.print("<TD>= <A HREF=\"" + className + "_attributes.html#" + name + "@" + i + "\" TARGET=\"Attributes\">" + str + "</TD>\n"); + printWriter.print("<TD>= <A HREF=\"" + className + "_attributes.html#" + Class2HTML.toHTML(name) + "@" + i + "\" TARGET=\"Attributes\">" + + Class2HTML.toHTML(str) + "</TD>\n"); break; } } @@ -114,7 +115,7 @@ final class MethodHTML { access = Utility.replace(access, " ", " "); final String htmlName = Class2HTML.toHTML(name); printWriter.print("<TR VALIGN=TOP><TD><FONT COLOR=\"#FF0000\"><A NAME=method" + methodNumber + ">" + access + "</A></FONT></TD>"); - printWriter.print("<TD>" + Class2HTML.referenceType(type) + "</TD><TD><A HREF=" + className + "_code.html#method" + methodNumber + " TARGET=Code>" + printWriter.print("<TD>" + Class2HTML.referenceType(type) + "</TD><TD><A HREF=\"" + className + "_code.html#method" + methodNumber + "\" TARGET=Code>" + htmlName + "</A></TD>\n<TD>("); for (int i = 0; i < args.length; i++) { printWriter.print(Class2HTML.referenceType(args[i])); diff --git a/src/test/java/org/apache/bcel/util/Class2HTMLXSSTest.java b/src/test/java/org/apache/bcel/util/Class2HTMLXSSTest.java index 478060a9..2e91f15d 100644 --- a/src/test/java/org/apache/bcel/util/Class2HTMLXSSTest.java +++ b/src/test/java/org/apache/bcel/util/Class2HTMLXSSTest.java @@ -56,4 +56,25 @@ class Class2HTMLXSSTest { assertFalse(methods.contains("\">x<script>"), "field name was emitted unescaped in text context"); assertTrue(methods.contains("<script>"), "expected the field name to be HTML-escaped"); } + + /** + * Quotes in attacker-controlled names must be escaped in attribute context (HREF/NAME values), or the name + * breaks out of the attribute and injects live event handlers. + */ + @Test + void testFieldNameQuotesAreEscapedInAttributeContext() throws Exception { + final ClassGen cg = new ClassGen("Evil2", "java.lang.Object", "Evil2.java", Const.ACC_PUBLIC, null); + cg.addField(new FieldGen(Const.ACC_PUBLIC, Type.INT, "x\" onmouseover=\"alert(1)", cg.getConstantPool()).getField()); + final JavaClass jc = cg.getJavaClass(); + + final File outputDir = new File("target/test-output/html-xss"); + if (!outputDir.mkdirs()) { + assertTrue(outputDir.isDirectory()); + } + new Class2HTML(jc, outputDir.getAbsolutePath() + File.separator); + + final String methods = new String(Files.readAllBytes(new File(outputDir, "Evil2_methods.html").toPath()), StandardCharsets.UTF_8); + assertFalse(methods.contains("x\" onmouseover"), "field name broke out of the attribute context"); + assertTrue(methods.contains("x" onmouseover"), "expected quotes in the field name to be escaped"); + } }
