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..7e0b95704c --- /dev/null +++ b/openam-core/src/test/java/org/openidentityplatform/openam/click/control/HiddenFieldTest.java @@ -0,0 +1,240 @@ +/* + * 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 static org.testng.Assert.assertThrows; +import static org.testng.Assert.assertTrue; + +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.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; + +/** + * 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 { + + /** 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. + */ + 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 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 { + + 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); + + // 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"); + assertNull(field.getValueObject(), + "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); + + 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. */ + @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"); + } + + /** + * 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("