From 6ec9f1f22c2c2726a150d6c7bcf640dca10a77e9 Mon Sep 17 00:00:00 2001 From: Lucian Boaca Date: Thu, 15 Oct 2026 10:23:44 +0000 Subject: [PATCH 1/6] Disable XML entity size limits when parsing watchface XML --- .../watchface/dfx/memory/WatchFaceData.kt | 15 ++++++++++-- .../dfx/memory/WatchFaceDataTest.java | 23 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt b/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt index 62c462b..bace5d8 100644 --- a/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt +++ b/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt @@ -16,7 +16,6 @@ package com.google.wear.watchface.dfx.memory -import org.w3c.dom.Document import java.io.ByteArrayInputStream import java.util.stream.Stream import javax.xml.parsers.DocumentBuilderFactory @@ -26,6 +25,7 @@ import javax.xml.transform.sax.SAXSource import javax.xml.xpath.XPathConstants import javax.xml.xpath.XPathFactory import kotlin.streams.asSequence +import org.w3c.dom.Document import org.xml.sax.InputSource internal class WatchFaceData private constructor() { @@ -88,7 +88,18 @@ internal class WatchFaceData private constructor() { val doc = docFactory.newDocumentBuilder().newDocument() val inputStream = InputSource(ByteArrayInputStream(xmlData)) val sax = SAXSource(inputStream) - val transformer = TransformerFactory.newInstance().newTransformer() + val transformer = + TransformerFactory.newInstance() + .apply { + try { + setAttribute("jdk.xml.maxGeneralEntitySizeLimit", 0) + setAttribute("jdk.xml.totalEntitySizeLimit", 0) + } catch (_: IllegalArgumentException) { + // Ignored on runtimes (e.g. Android) that do not support JDK XML limit + // attributes. + } + } + .newTransformer() transformer.transform(sax, DOMResult(doc)) return doc } diff --git a/play-validations/memory-footprint/src/test/java/com/google/wear/watchface/dfx/memory/WatchFaceDataTest.java b/play-validations/memory-footprint/src/test/java/com/google/wear/watchface/dfx/memory/WatchFaceDataTest.java index 499891f..613e600 100644 --- a/play-validations/memory-footprint/src/test/java/com/google/wear/watchface/dfx/memory/WatchFaceDataTest.java +++ b/play-validations/memory-footprint/src/test/java/com/google/wear/watchface/dfx/memory/WatchFaceDataTest.java @@ -7,11 +7,13 @@ import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.stream.Stream; import org.junit.Test; @@ -86,6 +88,27 @@ public void fromResourcesStream_createsPackageFromWindowsPaths() throws Exceptio } } + @Test + public void fromResourcesStream_parsesWatchFaceWithOver100kEntityReferences() { + String repeatedEntities = String.join("", Collections.nCopies(60000, ""a" ")); + String xml = + "" + + "" + + ""; + AndroidResource resource = + AndroidResource.fromPath( + Paths.get("base/res/raw/watchface.xml"), + xml.getBytes(StandardCharsets.UTF_8)); + + WatchFaceData watchFaceData = + WatchFaceData.fromResourcesStream( + Stream.of(resource), TEST_EVALUATION_SETTINGS, imageProcessor); + + assertThat(watchFaceData.getWatchFaceDocuments()).hasSize(1); + } + private AndroidResource readPackageFile(String path) throws RuntimeException { Path rootPath = Paths.get(TEST_PACKAGE_FILES_ROOT); Path filePath = Paths.get(TEST_PACKAGE_FILES_ROOT, path); From 35c19d82d9f3b13e7c2c14de81e7cc50c0e074b0 Mon Sep 17 00:00:00 2001 From: Lucian Boaca Date: Thu, 15 Oct 2026 10:32:09 +0000 Subject: [PATCH 2/6] Remove obsolete 'tools' package from setup-android in CI workflow --- .github/workflows/gradle.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index 14be170..67bb329 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -32,7 +32,7 @@ jobs: - name: Setup Android SDK uses: android-actions/setup-android@v3 with: - packages: "tools platform-tools build-tools;30.0.3" + packages: "platform-tools build-tools;30.0.3" - name: Get project hash run: | git_hash=$(git rev-parse --short "$GITHUB_SHA") @@ -84,7 +84,7 @@ jobs: - name: Setup Android SDK uses: android-actions/setup-android@v3 with: - packages: "tools platform-tools build-tools;30.0.3" + packages: "platform-tools build-tools;30.0.3" - name: Get project hash run: | git_hash=$(git rev-parse --short "$GITHUB_SHA") From d0e7bfbe5e7a1a69734b0f040e67a482d0d5f819 Mon Sep 17 00:00:00 2001 From: Lucian Boaca Date: Thu, 15 Oct 2026 10:49:16 +0000 Subject: [PATCH 3/6] Disable DTDs when disabling XML entity size limits --- .../watchface/dfx/memory/WatchFaceData.kt | 27 ++++++++++++++++++- .../dfx/memory/WatchFaceDataTest.java | 18 +++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt b/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt index bace5d8..4f60cbc 100644 --- a/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt +++ b/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt @@ -18,7 +18,9 @@ package com.google.wear.watchface.dfx.memory import java.io.ByteArrayInputStream import java.util.stream.Stream +import javax.xml.XMLConstants import javax.xml.parsers.DocumentBuilderFactory +import javax.xml.parsers.SAXParserFactory import javax.xml.transform.TransformerFactory import javax.xml.transform.dom.DOMResult import javax.xml.transform.sax.SAXSource @@ -27,6 +29,7 @@ import javax.xml.xpath.XPathFactory import kotlin.streams.asSequence import org.w3c.dom.Document import org.xml.sax.InputSource +import org.xml.sax.SAXException internal class WatchFaceData private constructor() { @@ -86,12 +89,34 @@ internal class WatchFaceData private constructor() { // DWF Runtime. See b/493476140 for details. val docFactory = DocumentBuilderFactory.newInstance().apply { isNamespaceAware = true } val doc = docFactory.newDocumentBuilder().newDocument() + val saxFactory = + try { + SAXParserFactory.newDefaultInstance() + } catch (_: NoSuchMethodError) { + SAXParserFactory.newInstance() + }.apply { + isNamespaceAware = true + setFeature("http://apache.org/xml/features/disallow-doctype-decl", true) + setFeature("http://xml.org/sax/features/external-general-entities", false) + setFeature("http://xml.org/sax/features/external-parameter-entities", false) + } + val xmlReader = + saxFactory.newSAXParser().xmlReader.apply { + try { + setProperty("jdk.xml.maxGeneralEntitySizeLimit", 0) + setProperty("jdk.xml.totalEntitySizeLimit", 0) + } catch (_: SAXException) { + // Ignored on parsers that do not support JDK XML limit properties. + } + } val inputStream = InputSource(ByteArrayInputStream(xmlData)) - val sax = SAXSource(inputStream) + val sax = SAXSource(xmlReader, inputStream) val transformer = TransformerFactory.newInstance() .apply { try { + setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "") + setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "") setAttribute("jdk.xml.maxGeneralEntitySizeLimit", 0) setAttribute("jdk.xml.totalEntitySizeLimit", 0) } catch (_: IllegalArgumentException) { diff --git a/play-validations/memory-footprint/src/test/java/com/google/wear/watchface/dfx/memory/WatchFaceDataTest.java b/play-validations/memory-footprint/src/test/java/com/google/wear/watchface/dfx/memory/WatchFaceDataTest.java index 613e600..b4fc160 100644 --- a/play-validations/memory-footprint/src/test/java/com/google/wear/watchface/dfx/memory/WatchFaceDataTest.java +++ b/play-validations/memory-footprint/src/test/java/com/google/wear/watchface/dfx/memory/WatchFaceDataTest.java @@ -1,6 +1,7 @@ package com.google.wear.watchface.dfx.memory; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -109,6 +110,23 @@ public void fromResourcesStream_parsesWatchFaceWithOver100kEntityReferences() { assertThat(watchFaceData.getWatchFaceDocuments()).hasSize(1); } + @Test + public void fromResourcesStream_rejectsXmlWithDoctype() { + String xml = + " ]>" + + ""; + AndroidResource resource = + AndroidResource.fromPath( + Paths.get("base/res/raw/watchface.xml"), + xml.getBytes(StandardCharsets.UTF_8)); + + assertThrows( + Exception.class, + () -> + WatchFaceData.fromResourcesStream( + Stream.of(resource), TEST_EVALUATION_SETTINGS, imageProcessor)); + } + private AndroidResource readPackageFile(String path) throws RuntimeException { Path rootPath = Paths.get(TEST_PACKAGE_FILES_ROOT); Path filePath = Paths.get(TEST_PACKAGE_FILES_ROOT, path); From 096aa2b7d110b6f0aacd05f3a0809397370d963f Mon Sep 17 00:00:00 2001 From: Lucian Boaca Date: Thu, 15 Oct 2026 10:55:00 +0000 Subject: [PATCH 4/6] Remove redundant TransformerFactory attributes --- .../wear/watchface/dfx/memory/WatchFaceData.kt | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt b/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt index 4f60cbc..08b04cd 100644 --- a/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt +++ b/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt @@ -18,7 +18,6 @@ package com.google.wear.watchface.dfx.memory import java.io.ByteArrayInputStream import java.util.stream.Stream -import javax.xml.XMLConstants import javax.xml.parsers.DocumentBuilderFactory import javax.xml.parsers.SAXParserFactory import javax.xml.transform.TransformerFactory @@ -111,20 +110,7 @@ internal class WatchFaceData private constructor() { } val inputStream = InputSource(ByteArrayInputStream(xmlData)) val sax = SAXSource(xmlReader, inputStream) - val transformer = - TransformerFactory.newInstance() - .apply { - try { - setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "") - setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "") - setAttribute("jdk.xml.maxGeneralEntitySizeLimit", 0) - setAttribute("jdk.xml.totalEntitySizeLimit", 0) - } catch (_: IllegalArgumentException) { - // Ignored on runtimes (e.g. Android) that do not support JDK XML limit - // attributes. - } - } - .newTransformer() + val transformer = TransformerFactory.newInstance().newTransformer() transformer.transform(sax, DOMResult(doc)) return doc } From dba669ed3df5f6fb7245f7865cc2c8530c318f4c Mon Sep 17 00:00:00 2001 From: Lucian Boaca Date: Thu, 15 Oct 2026 11:55:15 +0000 Subject: [PATCH 5/6] Pin GitHub Actions in gradle.yml to commit SHAs --- .github/workflows/gradle.yml | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index 67bb329..6f2a906 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -22,15 +22,15 @@ jobs: steps: - name: Checkout project - uses: actions/checkout@v3 + uses: actions/checkout@a37ce9120846195fa4ece8f58b268e6043cb2f26 # v3 - name: Set up JDK 17 - uses: actions/setup-java@v3 + uses: actions/setup-java@e9fbacdec3bb3b6036605a3e6f7995d66773a8c6 # v3 with: java-version: "17" distribution: "corretto" architecture: "x64" - name: Setup Android SDK - uses: android-actions/setup-android@v3 + uses: android-actions/setup-android@9fc6c4e9069bf8d3d10b2204b1fb8f6ef7065407 # v3 with: packages: "platform-tools build-tools;30.0.3" - name: Get project hash @@ -38,17 +38,17 @@ jobs: git_hash=$(git rev-parse --short "$GITHUB_SHA") echo "git_hash=$git_hash" >> $GITHUB_ENV - name: Check format - uses: gradle/actions/setup-gradle@v3 + uses: gradle/actions/setup-gradle@d9c87d481d55275bb5441eef3fe0e46805f9ef70 # v3 with: build-root-directory: play-validations arguments: spotlessCheck - name: Build memory-footprint.jar - uses: gradle/actions/setup-gradle@v3 + uses: gradle/actions/setup-gradle@d9c87d481d55275bb5441eef3fe0e46805f9ef70 # v3 with: build-root-directory: play-validations arguments: memory-footprint:jar -Dmemory-footprint.git_hash=${{ env.git_hash }} - name: Test memory-footprint - uses: gradle/actions/setup-gradle@v3 + uses: gradle/actions/setup-gradle@d9c87d481d55275bb5441eef3fe0e46805f9ef70 # v3 with: build-root-directory: play-validations arguments: memory-footprint:test @@ -57,15 +57,15 @@ jobs: steps: - name: Checkout project - uses: actions/checkout@v3 + uses: actions/checkout@a37ce9120846195fa4ece8f58b268e6043cb2f26 # v3 - name: Set up JDK 17 - uses: actions/setup-java@v3 + uses: actions/setup-java@e9fbacdec3bb3b6036605a3e6f7995d66773a8c6 # v3 with: java-version: "17" distribution: "corretto" architecture: "x64" - name: Build the validator - uses: gradle/actions/setup-gradle@v3 + uses: gradle/actions/setup-gradle@d9c87d481d55275bb5441eef3fe0e46805f9ef70 # v3 with: build-root-directory: third_party/wff arguments: :specification:validator:build @@ -74,15 +74,15 @@ jobs: steps: - name: Checkout project - uses: actions/checkout@v3 + uses: actions/checkout@a37ce9120846195fa4ece8f58b268e6043cb2f26 # v3 - name: Set up JDK 17 - uses: actions/setup-java@v3 + uses: actions/setup-java@e9fbacdec3bb3b6036605a3e6f7995d66773a8c6 # v3 with: java-version: "17" distribution: "corretto" architecture: "x64" - name: Setup Android SDK - uses: android-actions/setup-android@v3 + uses: android-actions/setup-android@9fc6c4e9069bf8d3d10b2204b1fb8f6ef7065407 # v3 with: packages: "platform-tools build-tools;30.0.3" - name: Get project hash @@ -90,17 +90,17 @@ jobs: git_hash=$(git rev-parse --short "$GITHUB_SHA") echo "git_hash=$git_hash" >> $GITHUB_ENV - name: Check format - uses: gradle/actions/setup-gradle@v3 + uses: gradle/actions/setup-gradle@d9c87d481d55275bb5441eef3fe0e46805f9ef70 # v3 with: build-root-directory: tools arguments: spotlessCheck - name: Build wff-optimizer.jar - uses: gradle/actions/setup-gradle@v3 + uses: gradle/actions/setup-gradle@d9c87d481d55275bb5441eef3fe0e46805f9ef70 # v3 with: build-root-directory: tools arguments: wff-optimizer:jar -Dmemory-footprint.git_hash=${{ env.git_hash }} - name: Test wff-optimizer - uses: gradle/actions/setup-gradle@v3 + uses: gradle/actions/setup-gradle@d9c87d481d55275bb5441eef3fe0e46805f9ef70 # v3 with: build-root-directory: tools arguments: wff-optimizer:test From 70dfa1e00cb0571ad2dd32cdec0d7d4c52a992c5 Mon Sep 17 00:00:00 2001 From: Lucian Boaca Date: Thu, 15 Oct 2026 11:58:42 +0000 Subject: [PATCH 6/6] Remove redundant isNamespaceAware on SAXParserFactory and document XXE/entity limits --- .../com/google/wear/watchface/dfx/memory/WatchFaceData.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt b/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt index 08b04cd..a3f940b 100644 --- a/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt +++ b/play-validations/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/WatchFaceData.kt @@ -94,7 +94,8 @@ internal class WatchFaceData private constructor() { } catch (_: NoSuchMethodError) { SAXParserFactory.newInstance() }.apply { - isNamespaceAware = true + // Disable DTDs and external entities to prevent XML External Entity (XXE) and + // recursive entity expansion (e.g. Billion Laughs) attacks on untrusted inputs. setFeature("http://apache.org/xml/features/disallow-doctype-decl", true) setFeature("http://xml.org/sax/features/external-general-entities", false) setFeature("http://xml.org/sax/features/external-parameter-entities", false) @@ -102,6 +103,9 @@ internal class WatchFaceData private constructor() { val xmlReader = saxFactory.newSAXParser().xmlReader.apply { try { + // Once DTDs and custom entities are disabled above, recursive entity + // expansion is impossible, so it is safe to disable these size limits to + // allow watch faces with a large number of built-in entities (like "). setProperty("jdk.xml.maxGeneralEntitySizeLimit", 0) setProperty("jdk.xml.totalEntitySizeLimit", 0) } catch (_: SAXException) {