[jira] [Created] (ZOOKEEPER-5097) Add configurable limits to snapshot deserialization (shared by file load, AdminServer restore, and learner sync)

["Prasath Jayasundar (Jira)" <ji...@apache.org>]

Prasath Jayasundar created ZOOKEEPER-5097:
---------------------------------------------

             Summary: Add configurable limits to snapshot deserialization (shared by file load, AdminServer restore, and learner sync)
                 Key: ZOOKEEPER-5097
                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-5097
             Project: ZooKeeper
          Issue Type: Bug
    Affects Versions: 3.9.6
            Reporter: Prasath Jayasundar


*Summary:* Add configurable limits to snapshot deserialization to prevent unbounded resource consumption

*Type:* Improvement
*Component:* server
*Affects:* 3.9.6 and master
----
Follow-up to a snapshot deserialization robustness discussion on security@zookeeper.apache.org. This was assessed as *not a vulnerability* under ZooKeeper's security model and is tracked here as hardening work.
h2. Background

The snapshot deserializers allocate based on values read from the input stream, with no aggregate byte, node, or entry budget. {{jute.maxbuffer}} bounds individual records but provides no cumulative limit, so a snapshot whose contents exceed the configured heap causes an {{OutOfMemoryError}} during load. This affects legitimate oversized snapshots as much as malformed ones — the parser cannot distinguish them without an operator-configured limit.

Three loops are unbounded:
 * {{ReferenceCountedACLCache.deserialize}} — declared ACL map count, accumulated into a {{{}LinkedHashMap{}}}, no validation of the count
 * {{DataTree.deserialize}} — sentinel-terminated ({{{}while (!"/".equals(path)){}}}), reads until the {{/}} sentinel or EOF
 * {{SerializeUtils.deserializeSnapshot}} — declared session count, unvalidated

Relevant failure detail: {{FileSnap.deserialize}} iterates candidate snapshots inside {{{}try \{ ... } catch (IOException e){}}}, so a malformed file is logged and skipped, but {{OutOfMemoryError}} is an {{Error}} rather than an {{IOException}} and propagates past that handler. With the JVM flags in the distribution's {{zkServer.sh}} ({{{}-XX:OnOutOfMemoryError='kill -9 %p'{}}}) the process is killed.
h2. Proposed work
 # A configurable size limit on the AdminServer restore request body.
 # Optional limits on entries during deserialization, failing with an {{IOException}} so that {{FileSnap.deserialize}} falls through to the next candidate snapshot rather than exhausting the heap.

h2. Where the limits belong

During scoping, a third call site was identified. {{Learner.syncWithLeader()}} deserializes the SNAP payload received from the leader through the same {{{}ZKDatabase{}}}/{{{}SerializeUtils{}}} path used by file load and AdminServer restore ({{{}Learner.java:601{}}}), with the sanity-check marker read only after deserialization completes ({{{}Learner.java:609-612{}}}).

There are therefore three entry points into the same deserializers:
||Entry point||Path||
|Snapshot file load|{{FileSnap.deserialize}}|
|AdminServer restore|{{ZooKeeperServer.restoreFromSnapshot}}|
|Learner sync|{{Learner.syncWithLeader}}|

Any limit should be implemented in the shared deserialization path rather than in {{SnapStream}} or on the restore request body, so that a single change covers all three. A bound placed in {{SnapStream}} would miss both restore and learner sync; a bound on the restore body would miss the other two.
h2. Design consideration: the fallback asymmetry

The rationale for failing with an {{IOException}} is that {{FileSnap.deserialize}} then skips to the next candidate snapshot. *That fallback does not exist on the learner sync path.* A learner that refuses the leader's snapshot will reconnect, be offered the same snapshot again, and be unable to rejoin the ensemble.

So the same exception produces graceful degradation on file load and an unjoinable learner on sync. Consequently:
 * Any new limit must default to {*}disabled, or to a generous value{*}.
 * When exceeded, it should fail with a clear error message {*}naming the configuration property involved{*}, so that an ensemble whose legitimate data has outgrown the limit fails in an obvious, fixable way rather than looping with an opaque failure.

----
 



--
This message was sent by Atlassian Jira
(v8.20.10#820010)