L1nq0 commented on PR #9075: URL: https://github.com/apache/storm/pull/9075#issuecomment-5565075222
@reiabreu Thanks for the sweep; all four points are in the revision, pushed as 54e4926. - maxarray (and maxdepth/maxrefs): added to the sample pattern in docs/SECURITY.md, now maxdepth=64;maxrefs=2097152;maxarray=1048576;maxbytes=10485760. maxarray sits well below maxbytes for the reason you describe: the array is checked when it is created, before its contents are read, so the byte limit alone would miss one big array. The docs next to the block now cover both properties, per-object counting and the overshoot case. - Merge vs replace: I took the merge branch of your either/or. The install now reads the stream's current filter and combines the two via ObjectInputFilter.merge, so a JVM-wide -Djdk.serialFilter set in worker.childopts is no longer quietly dropped on this path: if either filter rejects, the merged one rejects. GGraziadei raised the same point inline, with a JDK 25 check that matches. - Factory scope: your read on the extension point settled the question I'd left open on rzo1's thread (I had planned to try moving the install into SerializationFactory.getKryo()). You're right that it can't work generically: the factory only ever hands back a finished Kryo, with no way to reach a third-party factory's fallback path. Went with the pre-announced fallback: the Config javadoc and both docs now state the filter covers the default factory's fallback bridge, a custom topology.kryo.factory is on its own, and DefaultStateSerializer is not covered. - Per-object maxbytes: documented as such, in Serialization.md, SECURITY.md and the Config javadoc, together with the best-effort caveat above. Beyond your list, the same revision also took GGraziadei's inline points (constructor-injected filter instead of a public setter, and a length guard in SerializableSerializer.read() so the attacker-sized buffer is no longer allocated from an unvalidated length field on the tuple path), and rzo1's earlier points (empty default, submit-time validation of the pattern, Storm-owned test classes). -- 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]
