From 138bda1d795b3b8b2bee24236ab95f798b2b7466 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Wed, 19 Aug 2026 18:52:19 +0300 Subject: [PATCH 1/2] [GHSA-7j4m-m698-57hp] Remove the Java serialization round trip from the bundled Click HiddenField ClickUtils.decode(String) ran Base64, then GZIP, then ObjectInputStream.readObject() over its argument with no ObjectInputFilter, no class allowlist and no depth or size limit. Its only caller was HiddenField.bindRequestValue(), which reached it whenever the field's value class was a Serializable other than the handful the method parses explicitly - and the argument in that path is a request parameter. ClickUtils.encode produced the format decode read, and its only caller was HiddenField.render(). No page in the product binds such a field: nothing outside the Click framework calls addControl, the only Click HiddenField instances are the framework's own NonProcessedHiddenField over String and Long, and the pages the Click servlet serves are the configurator and upgrader, which create no controls. The sink therefore has no legitimate caller to protect, so it is removed rather than filtered: an ObjectInputFilter would leave a live readObject() behind a configuration, whereas deleting a closed round trip that nothing else produces or consumes cannot regress anything. A submission to a field declared with an unsupported value class now takes the same path every other unrecognised class already took - setValue(aValue), which refuses it because a String is not of the declared value class. The test builds the exact payload the removed encode() produced, wrapping a canary whose readObject records that it ran, and asserts the canary stays untouched when the field binds it. Against the previous code that test fails with the canary deserialized. com.sun.identity.config.util.TemplatedForm, which extended the upstream Click Form and was referenced from nowhere, is dropped at the same time: it was the one place from which a Click form could plausibly have been introduced. Reported by GitHub @leanworld7-netizen, and independently twice more. --- .../identity/config/util/TemplatedForm.java | 71 -------- .../openam/click/control/HiddenField.java | 39 ++--- .../openam/click/util/ClickUtils.java | 101 +---------- .../openam/click/control/HiddenFieldTest.java | 157 ++++++++++++++++++ 4 files changed, 172 insertions(+), 196 deletions(-) delete mode 100644 openam-core/src/main/java/com/sun/identity/config/util/TemplatedForm.java create mode 100644 openam-core/src/test/java/org/openidentityplatform/openam/click/control/HiddenFieldTest.java diff --git a/openam-core/src/main/java/com/sun/identity/config/util/TemplatedForm.java b/openam-core/src/main/java/com/sun/identity/config/util/TemplatedForm.java deleted file mode 100644 index d9c03bc22c..0000000000 --- a/openam-core/src/main/java/com/sun/identity/config/util/TemplatedForm.java +++ /dev/null @@ -1,71 +0,0 @@ -/** - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER. - * - * Copyright (c) 2007 Sun Microsystems Inc. All Rights Reserved - * - * The contents of this file are subject to the terms - * of the Common Development and Distribution License - * (the License). You may not use this file except in - * compliance with the License. - * - * You can obtain a copy of the License at - * https://opensso.dev.java.net/public/CDDLv1.0.html or - * opensso/legal/CDDLv1.0.txt - * See the License for the specific language governing - * permission and limitations under the License. - * - * When distributing Covered Code, include this CDDL - * Header Notice in each file and include the License file - * at opensso/legal/CDDLv1.0.txt. - * If applicable, add the following below the CDDL Header, - * with the fields enclosed by brackets [] replaced by - * your own identifying information: - * "Portions Copyrighted [year] [name of copyright owner]" - * - * $Id: TemplatedForm.java,v 1.4 2008/06/25 05:42:41 qcheng Exp $ - * - */ - -/* - * Portions Copyrighted 2011 ForgeRock AS - */ - -package com.sun.identity.config.util; - -import org.apache.click.control.Field; -import org.apache.click.control.Form; - -import java.util.Iterator; - -/** - * @author Jeffrey Bermudez - */ -public class TemplatedForm extends Form { - - public TemplatedForm(String name) { - super(name); - } - - public TemplatedForm() { - super(); - } - - public boolean doProcess() { - // We can overwrite this method to put some specific login here - return true; - } - - public final boolean onProcess() { - super.onProcess(); - - Iterator it = getFieldList().iterator(); - while (it.hasNext()) { - if (!((Field) it.next()).onProcess()) { - return false; - } - } - - return doProcess(); - } - -} diff --git a/openam-core/src/main/java/org/openidentityplatform/openam/click/control/HiddenField.java b/openam-core/src/main/java/org/openidentityplatform/openam/click/control/HiddenField.java index 836120c43a..82fedffbd3 100644 --- a/openam-core/src/main/java/org/openidentityplatform/openam/click/control/HiddenField.java +++ b/openam-core/src/main/java/org/openidentityplatform/openam/click/control/HiddenField.java @@ -19,11 +19,8 @@ */ package org.openidentityplatform.openam.click.control; -import org.openidentityplatform.openam.click.util.ClickUtils; import org.openidentityplatform.openam.click.util.HtmlStringBuffer; -import java.io.IOException; -import java.io.Serializable; import java.sql.Time; import java.sql.Timestamp; import java.util.Date; @@ -42,13 +39,11 @@ *
  • Long
    • *
  • Short
    • *
  • String
    • - *
  • Serializable
    • * *

    - * Serializable non-primitive objects will be serialized, compressed and - * Base64 encoded, using {@link ClickUtils#encode(Object)} - * method, and decoded using the corresponding - * {@link ClickUtils#decode(String)} method. + * Other value classes are not supported. Arbitrary Serializable objects used to be carried in + * the field as a Base64 encoded Java serialization stream and read back with + * ObjectInputStream.readObject(); that round-trip was removed under GHSA-7j4m-m698-57hp. * *

    HiddenField Example

    * @@ -313,18 +308,13 @@ public void bindRequestValue() { long time = Long.parseLong(aValue); setValueObject(new Date(time)); - } else if (Serializable.class.isAssignableFrom(valueClass)) { - try { - setValueObject(ClickUtils.decode(aValue)); - } catch (ClassNotFoundException cnfe) { - String msg = - "could not decode value for hidden field: " + aValue; - throw new RuntimeException(msg, cnfe); - } catch (IOException ioe) { - String msg = - "could not decode value for hidden field: " + aValue; - throw new RuntimeException(msg, ioe); - } + // GHSA-7j4m-m698-57hp: a further branch used to hand any other Serializable value + // class to ClickUtils.decode(), which ran ObjectInputStream.readObject() over a + // request parameter with no filter. Nothing in the product bound such a field, so the + // branch and the encode/decode pair behind it were removed rather than filtered. Such + // a submission now takes the same path as any other value class this method does not + // parse: setValue(aValue) below, which refuses it because a String is not of the + // declared value class. } else { setValue(aValue); } @@ -364,15 +354,6 @@ public void render(HtmlStringBuffer buffer) { String dateStr = String.valueOf(((Date) getValueObject()).getTime()); buffer.appendAttributeEscaped("value", dateStr); - } else if (getValueObject() instanceof Serializable) { - try { - buffer.appendAttribute("value", ClickUtils.encode(getValueObject())); - } catch (IOException ioe) { - String msg = - "could not encode value for hidden field: " - + getValueObject(); - throw new RuntimeException(msg, ioe); - } } else { buffer.appendAttributeEscaped("value", getValue()); } diff --git a/openam-core/src/main/java/org/openidentityplatform/openam/click/util/ClickUtils.java b/openam-core/src/main/java/org/openidentityplatform/openam/click/util/ClickUtils.java index 7e4c7533f4..c5ee075645 100644 --- a/openam-core/src/main/java/org/openidentityplatform/openam/click/util/ClickUtils.java +++ b/openam-core/src/main/java/org/openidentityplatform/openam/click/util/ClickUtils.java @@ -19,17 +19,12 @@ */ package org.openidentityplatform.openam.click.util; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.Closeable; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; import java.io.OutputStream; -import java.io.Serializable; import java.io.UnsupportedEncodingException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -48,7 +43,6 @@ import java.util.MissingResourceException; import java.util.ResourceBundle; import java.util.TreeMap; -import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; import jakarta.servlet.ServletContext; @@ -72,7 +66,6 @@ import org.apache.click.util.Format; import org.apache.click.util.MessagesMap; -import org.apache.commons.codec.binary.Base64; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.ClassUtils; import org.apache.commons.lang.StringUtils; @@ -1705,95 +1698,11 @@ public static void deployFileList(ServletContext servletContext, } - /** - * Return an encoded version of the Serializable object. The object - * will be serialized, compressed and Base 64 encoded. - * - * @param object the object to encode - * @return a serialized, compressed and Base 64 string encoding of the - * given object - * @throws IOException if an I/O error occurs - * @throws IllegalArgumentException if the object parameter is null, or if - * the object is not Serializable - */ - public static String encode(Object object) throws IOException { - if (object == null) { - throw new IllegalArgumentException("null object parameter"); - } - if (!(object instanceof Serializable)) { - throw new IllegalArgumentException("parameter not Serializable"); - } - - ByteArrayOutputStream bos = null; - GZIPOutputStream gos = null; - ObjectOutputStream oos = null; - - try { - bos = new ByteArrayOutputStream(); - gos = new GZIPOutputStream(bos); - oos = new ObjectOutputStream(gos); - - oos.writeObject(object); - - } finally { - close(oos); - close(gos); - close(bos); - } - - Base64 base64 = new Base64(); - - try { - byte[] byteData = base64.encode(bos.toByteArray()); - - return new String(byteData); - - } catch (Throwable t) { - String message = - "error occurred Base64 encoding: " + object + " : " + t; - throw new IOException(message); - } - } - - /** - * Return an object from the {@link #encode(Object)} string. - * - * @param string the encoded string - * @return an object from the encoded - * @throws ClassNotFoundException if the class could not be instantiated - * @throws IOException if an data I/O error occurs - */ - public static Object decode(String string) - throws ClassNotFoundException, IOException { - - Base64 base64 = new Base64(); - byte[] byteData = null; - - try { - byteData = base64.decode(string.getBytes()); - - } catch (Throwable t) { - String message = - "error occurred Base64 decoding: " + string + " : " + t; - throw new IOException(message); - } - - ByteArrayInputStream bis = null; - GZIPInputStream gis = null; - ObjectInputStream ois = null; - try { - bis = new ByteArrayInputStream(byteData); - gis = new GZIPInputStream(bis); - ois = new ObjectInputStream(gis); - - return ois.readObject(); - - } finally { - close(ois); - close(gis); - close(bis); - } - } + // GHSA-7j4m-m698-57hp: encode(Object)/decode(String) used to round-trip a Serializable + // through Base64 + GZIP + ObjectInputStream.readObject(), with no ObjectInputFilter and no + // class allowlist. Their only user was HiddenField, on both sides, and no page in the product + // bound a HiddenField to a value class that reached them - so rather than filter a sink with + // no legitimate caller, the pair and the HiddenField branches that used them were removed. /** * Builds a cookie string containing a username and password. diff --git a/openam-core/src/test/java/org/openidentityplatform/openam/click/control/HiddenFieldTest.java b/openam-core/src/test/java/org/openidentityplatform/openam/click/control/HiddenFieldTest.java new file mode 100644 index 0000000000..5ea18577bc --- /dev/null +++ b/openam-core/src/test/java/org/openidentityplatform/openam/click/control/HiddenFieldTest.java @@ -0,0 +1,157 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ + +package org.openidentityplatform.openam.click.control; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNull; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.io.Serializable; +import java.lang.reflect.Method; +import java.util.zip.GZIPOutputStream; + +import org.apache.commons.codec.binary.Base64; +import org.openidentityplatform.openam.click.util.ClickUtils; +import org.testng.annotations.Test; + +/** + * GHSA-7j4m-m698-57hp: {@code HiddenField} used to hand the value of a request parameter to + * {@code ClickUtils.decode()} - Base64, then GZIP, then {@code ObjectInputStream.readObject()} + * with no filter - whenever the field's value class was a {@code Serializable} other than the + * handful it parses explicitly. No page in the product bound such a field, so the branch and the + * encode/decode pair behind it were removed rather than filtered. + */ +public class HiddenFieldTest { + + /** + * Records whether Java deserialization ran. A gadget would use this moment to do something + * less polite. + */ + public static class Canary implements Serializable { + + private static final long serialVersionUID = 1L; + + static volatile boolean deserialized = false; + + private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { + in.defaultReadObject(); + deserialized = true; + } + } + + /** A field whose request value is supplied directly, so no Click Context is needed. */ + private static final class SubmittedHiddenField extends HiddenField { + + private final String requestValue; + + SubmittedHiddenField(String name, Class valueClass, String requestValue) { + super(name, valueClass); + this.requestValue = requestValue; + } + + @Override + protected String getRequestValue() { + return requestValue; + } + } + + @Test + public void aSerializableValueClassIsNoLongerDeserialized() throws Exception { + + Canary.deserialized = false; + String payload = legacyEncoding(new Canary()); + + SubmittedHiddenField field = new SubmittedHiddenField("canary", Canary.class, payload); + try { + field.bindRequestValue(); + } catch (IllegalArgumentException expected) { + // The submission now takes the same path as any other value class the control does + // not parse: it is refused, because a String is not of the declared value class. + } + + assertFalse(Canary.deserialized, + "the submitted serialization stream must not be deserialized"); + assertFalse(field.getValueObject() instanceof Canary, + "no object should have been reconstructed from the parameter"); + } + + /** The same submission, on a field declared to hold an interface type. */ + @Test + public void anInterfaceValueClassIsNoLongerDeserializedEither() throws Exception { + + Canary.deserialized = false; + String payload = legacyEncoding(new Canary()); + + SubmittedHiddenField field = new SubmittedHiddenField("canary", Serializable.class, payload); + try { + field.bindRequestValue(); + } catch (IllegalArgumentException expected) { + // as above + } + + assertFalse(Canary.deserialized, + "the submitted serialization stream must not be deserialized"); + } + + /** The round trip has no caller left, and no longer exists to acquire one. */ + @Test + public void clickUtilsNoLongerCarriesTheSerializationRoundTrip() { + + for (Method method : ClickUtils.class.getDeclaredMethods()) { + assertFalse("decode".equals(method.getName()) && method.getParameterCount() == 1 + && method.getParameterTypes()[0] == String.class, + "ClickUtils.decode(String) is an unfiltered readObject() sink and must stay removed"); + assertFalse("encode".equals(method.getName()) && method.getParameterCount() == 1 + && method.getParameterTypes()[0] == Object.class, + "ClickUtils.encode(Object) produced the format decode(String) read and must stay removed"); + } + } + + /** The value classes the control actually supports still bind. */ + @Test + public void supportedValueClassesStillBind() { + + SubmittedHiddenField text = new SubmittedHiddenField("text", String.class, "hello"); + text.bindRequestValue(); + assertEquals(text.getValueObject(), "hello"); + + SubmittedHiddenField number = new SubmittedHiddenField("number", Long.class, "42"); + number.bindRequestValue(); + assertEquals(number.getValueObject(), 42L); + + SubmittedHiddenField flag = new SubmittedHiddenField("flag", Boolean.class, "true"); + flag.bindRequestValue(); + assertEquals(flag.getValueObject(), Boolean.TRUE); + + SubmittedHiddenField empty = new SubmittedHiddenField("empty", Long.class, ""); + empty.bindRequestValue(); + assertNull(empty.getValueObject(), "an empty submission binds nothing"); + } + + /** Reproduces exactly what the removed {@code ClickUtils.encode(Object)} produced. */ + private static String legacyEncoding(Serializable object) throws IOException { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (ObjectOutputStream objects = new ObjectOutputStream(new GZIPOutputStream(bytes))) { + objects.writeObject(object); + } + return new String(new Base64().encode(bytes.toByteArray())); + } +} From 3a5a950c4ff0fb3b37951a9acd395a778a764068 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 20 Aug 2026 21:06:18 +0300 Subject: [PATCH 2/2] [GHSA-7j4m-m698-57hp] Pin the refusal and the render escaping in HiddenFieldTest The canary tests swallowed the IllegalArgumentException, so their assertion was also satisfied by a bind that never reached the branch under test: any future validation added earlier in bindRequestValue(), or a softening of the refusal to a log-and-return, would leave them green while testing nothing. They now assert the refusal itself, and that nothing was assigned to the field. render() is moved by this change and had no coverage. The branch the deleted Serializable arm used to intercept now lands on appendAttributeEscaped(), so a value class the control does not parse is rendered and its value attribute asserted escaped there, with String, Long and Date covering the rest. Four of the seven cases now fail against the pre-change sources, up from three: the added render case emits the raw Base64 serialization stream unescaped. --- .../openam/click/control/HiddenFieldTest.java | 107 ++++++++++++++++-- 1 file changed, 95 insertions(+), 12 deletions(-) diff --git a/openam-core/src/test/java/org/openidentityplatform/openam/click/control/HiddenFieldTest.java b/openam-core/src/test/java/org/openidentityplatform/openam/click/control/HiddenFieldTest.java index 5ea18577bc..7e0b95704c 100644 --- a/openam-core/src/test/java/org/openidentityplatform/openam/click/control/HiddenFieldTest.java +++ b/openam-core/src/test/java/org/openidentityplatform/openam/click/control/HiddenFieldTest.java @@ -19,6 +19,8 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertThrows; +import static org.testng.Assert.assertTrue; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -26,10 +28,12 @@ import java.io.ObjectOutputStream; import java.io.Serializable; import java.lang.reflect.Method; +import java.util.Date; import java.util.zip.GZIPOutputStream; import org.apache.commons.codec.binary.Base64; import org.openidentityplatform.openam.click.util.ClickUtils; +import org.openidentityplatform.openam.click.util.HtmlStringBuffer; import org.testng.annotations.Test; /** @@ -41,6 +45,13 @@ */ public class HiddenFieldTest { + /** A payload that must never reach the rendered markup unescaped. */ + private static final String XSS = "\">"; + + /** The same payload as {@code HtmlStringBuffer.appendAttributeEscaped()} writes it. */ + private static final String XSS_ESCAPED = + ""><script>alert(1)</script>"; + /** * Records whether Java deserialization ran. A gadget would use this moment to do something * less polite. @@ -57,6 +68,22 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE } } + /** + * A Serializable value class the control does not parse. Its rendering used to be intercepted + * by the removed {@code instanceof Serializable} arm, which wrote + * {@code ClickUtils.encode(...)} through the unescaped {@code appendAttribute}; it now falls + * to the escaping branch at the end of {@link HiddenField#render(HtmlStringBuffer)}. + */ + public static class Unsupported implements Serializable { + + private static final long serialVersionUID = 1L; + + @Override + public String toString() { + return XSS; + } + } + /** A field whose request value is supplied directly, so no Click Context is needed. */ private static final class SubmittedHiddenField extends HiddenField { @@ -80,16 +107,16 @@ public void aSerializableValueClassIsNoLongerDeserialized() throws Exception { String payload = legacyEncoding(new Canary()); SubmittedHiddenField field = new SubmittedHiddenField("canary", Canary.class, payload); - try { - field.bindRequestValue(); - } catch (IllegalArgumentException expected) { - // The submission now takes the same path as any other value class the control does - // not parse: it is refused, because a String is not of the declared value class. - } + + // The submission takes the same path as any other value class the control does not parse: + // it is refused, because a String is not of the declared value class. Asserting that the + // refusal happens keeps the canary assertion below from being satisfied by a bind that + // never reached the branch under test. + assertThrows(IllegalArgumentException.class, field::bindRequestValue); assertFalse(Canary.deserialized, "the submitted serialization stream must not be deserialized"); - assertFalse(field.getValueObject() instanceof Canary, + assertNull(field.getValueObject(), "no object should have been reconstructed from the parameter"); } @@ -101,14 +128,13 @@ public void anInterfaceValueClassIsNoLongerDeserializedEither() throws Exception String payload = legacyEncoding(new Canary()); SubmittedHiddenField field = new SubmittedHiddenField("canary", Serializable.class, payload); - try { - field.bindRequestValue(); - } catch (IllegalArgumentException expected) { - // as above - } + + assertThrows(IllegalArgumentException.class, field::bindRequestValue); assertFalse(Canary.deserialized, "the submitted serialization stream must not be deserialized"); + assertNull(field.getValueObject(), + "no object should have been reconstructed from the parameter"); } /** The round trip has no caller left, and no longer exists to acquire one. */ @@ -146,6 +172,63 @@ public void supportedValueClassesStillBind() { assertNull(empty.getValueObject(), "an empty submission binds nothing"); } + /** + * The branch the removed {@code instanceof Serializable} arm used to intercept. This is the + * one rendering path this change moved, and the escaping call it lands on is what stands + * between a value class the control does not parse and its markup. + */ + @Test + public void renderEscapesAValueClassTheControlDoesNotParse() { + + HiddenField field = new HiddenField("unsupported", Unsupported.class); + field.setValueObject(new Unsupported()); + + HtmlStringBuffer buffer = new HtmlStringBuffer(); + field.render(buffer); + + assertFalse(buffer.toString().contains("