From 0c629ddebf821a9f0f605d9b99ffff9e493d6cb8 Mon Sep 17 00:00:00 2001 From: Seigo Nonaka Date: Wed, 24 Feb 2021 20:13:38 -0800 Subject: [PATCH] Avoid infinite loop for invalid XML format Bug: 180364777 Test: atest FontListParserTest Change-Id: I56de88c6b424d84c844dfdc121eea044e1ac6aad --- .../android/graphics/FontListParserTest.java | 109 +++++++++++++++++- .../java/android/graphics/FontListParser.java | 14 ++- 2 files changed, 119 insertions(+), 4 deletions(-) diff --git a/core/tests/coretests/src/android/graphics/FontListParserTest.java b/core/tests/coretests/src/android/graphics/FontListParserTest.java index eae41e37e5f38..7bc81cd2f9280 100644 --- a/core/tests/coretests/src/android/graphics/FontListParserTest.java +++ b/core/tests/coretests/src/android/graphics/FontListParserTest.java @@ -26,6 +26,8 @@ import static android.text.FontConfig.FontFamily.VARIANT_ELEGANT; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static junit.framework.Assert.fail; + import android.graphics.fonts.FontStyle; import android.os.LocaleList; import android.text.FontConfig; @@ -44,6 +46,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; +import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.Arrays; @@ -221,9 +224,113 @@ public final class FontListParserTest { .that(readFamily(serialized)).isEqualTo(expected); } + @Test + public void invalidXml_unpaired_family() throws Exception { + String xml = "" + + "" + + " " + + " test.ttc" + + ""; + + try (InputStream is = new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8))) { + FontListParser.parse(is); + fail(); + } catch (IOException | XmlPullParserException e) { + // pass + } + } + + @Test + public void invalidXml_unpaired_font() throws Exception { + String xml = "" + + "" + + " " + + " test.ttc" + + " " + + ""; + + try (InputStream is = new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8))) { + FontListParser.parse(is); + fail(); + } catch (IOException | XmlPullParserException e) { + // pass + } + } + + @Test + public void invalidXml_unpaired_axis() throws Exception { + String xml = "" + + "" + + " " + + " test.ttc" + + " " + + " " + + " " + + ""; + + try (InputStream is = new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8))) { + FontListParser.parse(is); + fail(); + } catch (IOException | XmlPullParserException e) { + // pass + } + } + + @Test + public void invalidXml_unclosed_family() throws Exception { + String xml = "" + + "" + + " test.ttc" + + " " + + ""; + + try (InputStream is = new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8))) { + FontListParser.parse(is); + fail(); + } catch (IOException | XmlPullParserException e) { + // pass + } + } + + @Test + public void invalidXml_unclosed_font() throws Exception { + String xml = "" + + "" + + " " + + " " + + ""; + + try (InputStream is = new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8))) { + FontListParser.parse(is); + fail(); + } catch (IOException | XmlPullParserException e) { + // pass + } + } + + @Test + public void invalidXml_unclosed_axis() throws Exception { + String xml = "" + + "" + + " " + + " test.ttc" + + " " + + " " + + ""; + + try (InputStream is = new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8))) { + FontListParser.parse(is); + fail(); + } catch (IOException | XmlPullParserException e) { + // pass + } + } + private FontConfig.FontFamily readFamily(String xml) throws IOException, XmlPullParserException { - StandardCharsets.UTF_8.name(); ByteArrayInputStream buffer = new ByteArrayInputStream( xml.getBytes(StandardCharsets.UTF_8)); XmlPullParser parser = Xml.newPullParser(); diff --git a/graphics/java/android/graphics/FontListParser.java b/graphics/java/android/graphics/FontListParser.java index 63df0db99764d..95c7715a16884 100644 --- a/graphics/java/android/graphics/FontListParser.java +++ b/graphics/java/android/graphics/FontListParser.java @@ -136,7 +136,7 @@ public class FontListParser { customization.getAdditionalNamedFamilies(); parser.require(XmlPullParser.START_TAG, null, "familyset"); - while (parser.next() != XmlPullParser.END_TAG) { + while (keepReading(parser)) { if (parser.getEventType() != XmlPullParser.START_TAG) continue; String tag = parser.getName(); if (tag.equals("family")) { @@ -158,6 +158,12 @@ public class FontListParser { return new FontConfig(families, aliases, lastModifiedDate, configVersion); } + private static boolean keepReading(XmlPullParser parser) + throws XmlPullParserException, IOException { + int next = parser.next(); + return next != XmlPullParser.END_TAG && next != XmlPullParser.END_DOCUMENT; + } + /** * Read family tag in fonts.xml or oem_customization.xml */ @@ -168,7 +174,7 @@ public class FontListParser { final String lang = parser.getAttributeValue("", "lang"); final String variant = parser.getAttributeValue(null, "variant"); final List fonts = new ArrayList<>(); - while (parser.next() != XmlPullParser.END_TAG) { + while (keepReading(parser)) { if (parser.getEventType() != XmlPullParser.START_TAG) continue; final String tag = parser.getName(); if (tag.equals(TAG_FONT)) { @@ -232,7 +238,7 @@ public class FontListParser { boolean isItalic = STYLE_ITALIC.equals(parser.getAttributeValue(null, ATTR_STYLE)); String fallbackFor = parser.getAttributeValue(null, ATTR_FALLBACK_FOR); StringBuilder filename = new StringBuilder(); - while (parser.next() != XmlPullParser.END_TAG) { + while (keepReading(parser)) { if (parser.getEventType() == XmlPullParser.TEXT) { filename.append(parser.getText()); } @@ -359,6 +365,8 @@ public class FontListParser { case XmlPullParser.END_TAG: depth--; break; + case XmlPullParser.END_DOCUMENT: + return; } } }