lprimak commented on code in PR #2832:
URL: https://github.com/apache/shiro/pull/2832#discussion_r3575609247
##########
core/src/main/java/org/apache/shiro/mgt/AbstractRememberMeManager.java:
##########
@@ -119,9 +152,17 @@ protected ObjectInputStream
createObjectInputStream(InputStream inputStream) thr
/**
* Default constructor that initializes a {@link DefaultSerializer} as the
{@link #getSerializer() serializer} and
* an {@link AesCipherService} as the {@link #getCipherService()
cipherService}.
+ * <p/>
+ * As defense-in-depth against the {@link
#getRememberedPrincipals(SubjectContext)} deserialization path
+ * operating on untrusted, client-supplied input, the default serializer
is also pre-configured with the
+ * {@link #DEFAULT_OBJECT_INPUT_FILTER_PATTERN conservative resource-limit
ObjectInputFilter} described above.
*/
+ @SuppressWarnings("unchecked")
public AbstractRememberMeManager() {
setCipherKey(((AesCipherService)
cipherService).generateNewKey().getEncoded());
+ if (serializer instanceof DefaultSerializer) {
Review Comment:
Pull up to `Serializer` API, there should be no need for cast
##########
core/src/main/java/org/apache/shiro/mgt/AbstractRememberMeManager.java:
##########
@@ -91,6 +92,38 @@ protected ClassLoader doGetClassLoader() {
}
};
+ /**
+ * Default <a href="https://openjdk.org/jeps/290";>JEP-290</a> filter
pattern applied to the
+ * {@link #getSerializer() serializer}'s {@code ObjectInputStream} when
deserializing the RememberMe cookie
+ * payload, an untrusted, client-supplied value (see {@link
#getRememberedPrincipals(SubjectContext)}).
+ * <p/>
+ * This default is deliberately a <em>resource-limit-only</em> filter: it
bounds the object graph depth,
+ * array size, back-reference count, and total stream size that {@code
readObject()} will process, but it
+ * does not restrict <em>which</em> classes may be deserialized. It is
intended as defense-in-depth against
+ * oversized or deeply nested (denial-of-service shaped) payloads reaching
this deserialization sink. It is
+ * <em>not</em> protection against remote-code-execution gadget chains:
typical serialization gadget chains
+ * (for example the Apache Commons Collections family) are shallow and
small, so they stay well within these
+ * limits and are <em>not</em> rejected by them. Stopping such chains
requires a class-based allow-list,
+ * which cannot be a safe default here because principal types are
entirely application-defined (custom
+ * {@code Serializable} principal classes are common) and a default
allow-list would break existing
+ * deployments. Applications that need that stronger, class-based defense
(for example to reduce the blast
+ * radius of a leaked or static cipher key, the classic Shiro-550 /
CVE-2016-4437 scenario) can configure
+ * one in a single line; see {@link #getSerializer()}. The depth limit is
set generously (well above the
+ * depth of realistic principal object graphs) so that well-formed
principal data is not rejected.
+ *
+ * @since 3.1
Review Comment:
Please replace all occurrences with `@since 3.0.1`
--
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]
