diff --git a/THREAT-MODEL.md b/THREAT-MODEL.md index eb3dd23..d16b8ed 100644 --- a/THREAT-MODEL.md +++ b/THREAT-MODEL.md @@ -284,25 +284,33 @@ overload disables both. The pre-parsed-`Element` / `XMLStreamReader` / `org.apache.neethi.parser.maxDepth=256`, `org.apache.neethi.parser.maxElements=100000`, `org.apache.neethi.parser.maxAttributes=10000` *(documented: - `README.txt`)*. + `README.txt` — implemented in `PolicyBuilder.java`, enforced + operator-walk and Stax-to-DOM materialization phases)*. - Documented bound on remotely fetched policy size: `org.apache.neethi.remote.maxPolicyBytes=67108864` bytes (`64 MiB`) - *(documented: `README.txt`)*. + *(documented: `README.txt` — bounds total bytes read, not element + budget; assertion subtrees from remote policies are still charged + against `maxElements` / `maxAttributes`)*. - Documented hard cap of `10000` normalized policy alternatives during normalization/intersection *(documented: `README.txt`)*. - Documented hard cap of `1000000` assertion-pair intersection attempts per top-level `PolicyIntersector.intersect(...)` / `PolicyIntersector.compatiblePolicies(...)` call *(documented: `README.txt`)*. -- No documented bound on the number of `PolicyReference` URIs in a - single policy *(inferred — §14 Q7)*. Transitive fetch chains cannot +- **No documented bound on the number of `PolicyReference` URIs in a + single policy** *(inferred — §14 Q7)*. Transitive fetch chains cannot occur inside one normalization call: a fetched policy is re-normalized through the registry-only path, which throws on any - further unresolved absolute reference *(inferred — §14 Q11)*. -- No rate limit on `getRemoteReferencedPolicy` fetches — each direct + further unresolved absolute reference *(inferred — §14 Q11)*. Note: + the element/attribute budgets (P3, P4) apply to all fetched policies + as well as local policies, so an embedded attacker cannot bypass them + via chained remote references. +- **No rate limit on `getRemoteReferencedPolicy` fetches** — each direct embedder call to `PolicyReference.normalize(reg, deep)` on an unresolved reference triggers one HTTP GET; `Policy.normalize(...)` itself issues no fetches and throws on an unresolved reference. + Multiple fetches can amplify wall-clock latency but cannot amplify + memory consumption beyond `maxElements` / `maxAttributes`. - Connect-timeout (5 s) and read-timeout (10 s) bound the wall-clock per fetch, but an embedder that directly dereferences many distinct unresolved references can multiply the latency *(inferred — §14 Q7)*. @@ -347,23 +355,32 @@ overload disables both. The pre-parsed-`Element` / `XMLStreamReader` / - *(documented: `README.txt` — `org.apache.neethi.parser.maxDepth`, default `256`)* -### P3 — Parsed-element-count budget is enforced +### P3 — Parsed-element-count budget is enforced on all materialized elements - **Condition**: policy parsing occurs through Neethi's parser path. + This includes the operator walk over `wsp:*` elements **and** the + Stax-to-DOM conversion of assertion subtrees (`StaxToDOMConverter`), + which materializes XML stream events into DOM nodes. - **Violation symptom**: parser accepts more than configured/default - `org.apache.neethi.parser.maxElements` elements. + `org.apache.neethi.parser.maxElements` elements from *any* materialization + phase (operator walk or Stax conversion). - **Severity**: **availability-relevant**, `VALID` per §13. - *(documented: `README.txt` — `org.apache.neethi.parser.maxElements`, - default `100000`)* + default `100000`; implemented in `PolicyBuilder.java`, + `StaxToDOMConverter.java`)* -### P4 — Parsed-attribute-count budget is enforced +### P4 — Parsed-attribute-count budget is enforced on all materialized attributes - **Condition**: policy parsing occurs through Neethi's parser path. + This includes attributes on `wsp:*` operators **and** attributes on + assertion elements materialized by Stax-to-DOM conversion. - **Violation symptom**: parser accepts more than configured/default - `org.apache.neethi.parser.maxAttributes` attributes. + `org.apache.neethi.parser.maxAttributes` attributes from *any* materialization + phase (operator attributes or Stax-to-DOM conversion). - **Severity**: **availability-relevant**, `VALID` per §13. - *(documented: `README.txt` — `org.apache.neethi.parser.maxAttributes`, - default `10000`)* + default `10000`; implemented in `PolicyBuilder.java`, + `StaxToDOMConverter.java`)* ### P5 — Remote policy fetch byte budget is enforced diff --git a/src/main/java/org/apache/neethi/PolicyBuilder.java b/src/main/java/org/apache/neethi/PolicyBuilder.java index e208fc9..ee6c9c8 100644 --- a/src/main/java/org/apache/neethi/PolicyBuilder.java +++ b/src/main/java/org/apache/neethi/PolicyBuilder.java @@ -266,6 +266,14 @@ private PolicyOperator processOperationElement(Object operationElement, for (Map.Entry ent : attributes.entrySet()) { policyOperator.addAttribute(ent.getKey(), ent.getValue()); } + } else if (Constants.TYPE_POLICY != operator.getType()) { + // attributes on ExactlyOne/All operator elements were previously + // exempt from the maxAttributes budget + Map operatorAttributes = + factory.getConverterRegistry().getAttributes(operationElement); + if (operatorAttributes != null) { + context.incrementAttributeCount(operatorAttributes.size()); + } } for (Iterator iterator = factory.getConverterRegistry().getChildElements(operationElement); @@ -292,10 +300,12 @@ private PolicyOperator processOperationElement(Object operationElement, // a nested wsp:Policy inside this assertion re-enters // getPolicy below the assertion element itself context.recordReentryDepth(depth + 2); + context.incrementElementCount(); operator.addPolicyComponent(factory.build(childElement)); } } else { context.recordReentryDepth(depth + 2); + context.incrementElementCount(); operator.addPolicyComponent(factory.build(childElement)); } } @@ -369,6 +379,31 @@ void incrementAttributeCount(int delta) { } } + /** + * Charges one materialized element against the budget of the policy parse + * in progress on the current thread, if any. Called by the converter + * layer when it copies an assertion subtree into a new representation, so + * that maxElements bounds every node materialized on behalf of a single + * top-level parse - not only wsp:* operator elements. + */ + public static void chargeAmbientElement() { + ParseBudgetContext context = CURRENT_BUDGET.get(); + if (context != null) { + context.incrementElementCount(); + } + } + + /** + * Charges {@code count} materialized attributes against the budget of the + * policy parse in progress on the current thread, if any. + */ + public static void chargeAmbientAttributes(int count) { + ParseBudgetContext context = CURRENT_BUDGET.get(); + if (context != null) { + context.incrementAttributeCount(count); + } + } + protected void notifyUnknownPolicyElement(Object childElement) { //NO-Op - subclass could log or throw exception or something } diff --git a/src/main/java/org/apache/neethi/builders/converters/StaxToDOMConverter.java b/src/main/java/org/apache/neethi/builders/converters/StaxToDOMConverter.java index dff0006..9047725 100644 --- a/src/main/java/org/apache/neethi/builders/converters/StaxToDOMConverter.java +++ b/src/main/java/org/apache/neethi/builders/converters/StaxToDOMConverter.java @@ -34,6 +34,8 @@ import org.w3c.dom.Element; import org.w3c.dom.Node; +import org.apache.neethi.PolicyBuilder; + /** * */ @@ -72,6 +74,12 @@ public static void readDocElements(Document doc, Node parent, } e = (Element)parent.appendChild(e); + // charge every node materialized for an assertion subtree + // against the budget of the parse that requested it; the + // operator walk in PolicyBuilder never sees these nodes + PolicyBuilder.chargeAmbientElement(); + PolicyBuilder.chargeAmbientAttributes(reader.getAttributeCount()); + for (int ns = 0; ns < reader.getNamespaceCount(); ns++) { String uri = reader.getNamespaceURI(ns); String prefix = reader.getNamespacePrefix(ns); diff --git a/src/test/java/org/apache/neethi/PolicyBuilderAssertionSubtreeBudgetTest.java b/src/test/java/org/apache/neethi/PolicyBuilderAssertionSubtreeBudgetTest.java new file mode 100644 index 0000000..f47e3c4 --- /dev/null +++ b/src/test/java/org/apache/neethi/PolicyBuilderAssertionSubtreeBudgetTest.java @@ -0,0 +1,98 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.neethi; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; + +import org.junit.Test; + +/** + * The maxElements/maxAttributes budgets used to fire only on the wsp:* + * operator walk: every node inside an assertion subtree was materialized into + * DOM by the converter layer with no counting at all, so millions of elements + * could be allocated while the counters reported single digits. These tests + * pin the fix: assertion subtrees are charged against the same parse budget. + */ +public class PolicyBuilderAssertionSubtreeBudgetTest extends PolicyTestCase { + + @Test + public void testAssertionSubtreeElementsAreCountedAgainstBudget() { + System.setProperty(PolicyBuilder.MAX_ELEMENTS_PROPERTY, "500"); + try { + PolicyBuilder builder = new PolicyBuilder(); + try { + builder.getPolicy(xmlStream(buildWideAssertionPolicyXml(600, 0))); + fail("Expected RuntimeException due to element budget"); + } catch (RuntimeException ex) { + assertTrue(ex.getMessage().contains("maximum number of elements")); + } + } finally { + System.clearProperty(PolicyBuilder.MAX_ELEMENTS_PROPERTY); + } + } + + @Test + public void testAssertionSubtreeAttributesAreCountedAgainstBudget() { + System.setProperty(PolicyBuilder.MAX_ATTRIBUTES_PROPERTY, "500"); + try { + PolicyBuilder builder = new PolicyBuilder(); + try { + builder.getPolicy(xmlStream(buildWideAssertionPolicyXml(200, 5))); + fail("Expected RuntimeException due to attribute budget"); + } catch (RuntimeException ex) { + assertTrue(ex.getMessage().contains("maximum number of attributes")); + } + } finally { + System.clearProperty(PolicyBuilder.MAX_ATTRIBUTES_PROPERTY); + } + } + + @Test + public void testSmallAssertionSubtreeParses() { + PolicyBuilder builder = new PolicyBuilder(); + Policy policy = builder.getPolicy(xmlStream(buildWideAssertionPolicyXml(10, 2))); + + assertNotNull(policy); + } + + private static InputStream xmlStream(String xml) { + return new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8)); + } + + private static String buildWideAssertionPolicyXml(int elements, int attributesPerElement) { + StringBuilder xml = new StringBuilder(256 + elements * (24 + attributesPerElement * 12)); + xml.append("") + .append(""); + + for (int i = 0; i < elements; i++) { + xml.append(""); + } + + xml.append("") + .append(""); + return xml.toString(); + } +}