feat(api): add JadxArgs property to adjust xml security checks (#2291)

This commit is contained in:
Skylot
2024-10-09 20:48:15 +01:00
parent 2d10537050
commit 063af8cd62
24 changed files with 271 additions and 209 deletions
@@ -30,6 +30,9 @@ import jadx.api.impl.AnnotatedCodeWriter;
import jadx.api.impl.InMemoryCodeCache;
import jadx.api.plugins.loader.JadxBasePluginLoader;
import jadx.api.plugins.loader.JadxPluginLoader;
import jadx.api.security.IJadxSecurity;
import jadx.api.security.JadxSecurityFlag;
import jadx.api.security.impl.JadxSecurity;
import jadx.api.usage.IUsageInfoCache;
import jadx.api.usage.impl.InMemoryUsageInfoCache;
import jadx.core.deobf.DeobfAliasProvider;
@@ -174,6 +177,11 @@ public class JadxArgs implements Closeable {
*/
private IJadxFilesGetter filesGetter = TempFilesGetter.INSTANCE;
/**
* Additional data validation and security checks
*/
private IJadxSecurity security = new JadxSecurity(JadxSecurityFlag.all());
/**
* Don't save files (can be using for performance testing)
*/
@@ -726,6 +734,14 @@ public class JadxArgs implements Closeable {
this.filesGetter = filesGetter;
}
public IJadxSecurity getSecurity() {
return security;
}
public void setSecurity(IJadxSecurity security) {
this.security = security;
}
public boolean isSkipFilesSave() {
return skipFilesSave;
}
@@ -51,7 +51,6 @@ import jadx.core.utils.Utils;
import jadx.core.utils.exceptions.JadxRuntimeException;
import jadx.core.utils.files.FileUtils;
import jadx.core.utils.tasks.TaskExecutor;
import jadx.core.xmlgen.BinaryXMLParser;
import jadx.core.xmlgen.ResourcesSaver;
/**
@@ -92,8 +91,6 @@ public final class JadxDecompiler implements Closeable {
private List<JavaClass> classes;
private List<ResourceFile> resources;
private BinaryXMLParser binaryXmlParser;
private final IDecompileScheduler decompileScheduler = new DecompilerScheduler();
private final JadxEventsImpl events = new JadxEventsImpl();
private final ResourcesLoader resourcesLoader = new ResourcesLoader(this);
@@ -168,7 +165,6 @@ public final class JadxDecompiler implements Closeable {
root = null;
classes = null;
resources = null;
binaryXmlParser = null;
events.reset();
}
@@ -467,13 +463,6 @@ public final class JadxDecompiler implements Closeable {
return root;
}
synchronized BinaryXMLParser getBinaryXmlParser() {
if (binaryXmlParser == null) {
binaryXmlParser = new BinaryXMLParser(root);
}
return binaryXmlParser;
}
/**
* Get JavaClass by ClassNode without loading and decompilation
*/
@@ -0,0 +1,20 @@
package jadx.api.security;
import java.io.InputStream;
import org.w3c.dom.Document;
public interface IJadxSecurity {
/**
* Check if application package is safe
*
* @return normalized/sanitized string or same string if safe
*/
String verifyAppPackage(String appPackage);
/**
* XML document parser
*/
Document parseXml(InputStream in);
}
@@ -0,0 +1,18 @@
package jadx.api.security;
import java.util.EnumSet;
import java.util.Set;
public enum JadxSecurityFlag {
VERIFY_APP_PACKAGE,
SECURE_XML_PARSER;
public static Set<JadxSecurityFlag> all() {
return EnumSet.allOf(JadxSecurityFlag.class);
}
public static Set<JadxSecurityFlag> none() {
return EnumSet.noneOf(JadxSecurityFlag.class);
}
}
@@ -0,0 +1,73 @@
package jadx.api.security.impl;
import java.io.InputStream;
import java.util.Set;
import javax.xml.parsers.DocumentBuilderFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.w3c.dom.Document;
import jadx.api.security.IJadxSecurity;
import jadx.api.security.JadxSecurityFlag;
import jadx.core.deobf.NameMapper;
public class JadxSecurity implements IJadxSecurity {
private static final Logger LOG = LoggerFactory.getLogger(JadxSecurity.class);
private final Set<JadxSecurityFlag> flags;
public JadxSecurity(Set<JadxSecurityFlag> flags) {
this.flags = flags;
}
@Override
public String verifyAppPackage(String appPackage) {
if (flags.contains(JadxSecurityFlag.VERIFY_APP_PACKAGE)
&& !NameMapper.isValidFullIdentifier(appPackage)) {
LOG.warn("App package '{}' has invalid format and will be ignored", appPackage);
return "INVALID_PACKAGE";
}
return appPackage;
}
@Override
public Document parseXml(InputStream in) {
DocumentBuilderFactory dbf;
if (flags.contains(JadxSecurityFlag.SECURE_XML_PARSER)) {
dbf = SecureDBFHolder.INSTANCE;
} else {
dbf = SimpleDBFHolder.INSTANCE;
}
try {
return dbf.newDocumentBuilder().parse(in);
} catch (Exception e) {
throw new RuntimeException("Failed to parse xml", e);
}
}
private static final class SimpleDBFHolder {
private static final DocumentBuilderFactory INSTANCE = DocumentBuilderFactory.newInstance();
}
private static final class SecureDBFHolder {
private static final DocumentBuilderFactory INSTANCE = buildSecureDBF();
private static DocumentBuilderFactory buildSecureDBF() {
try {
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
dbf.setFeature("http://apache.org/xml/features/dom/create-entity-ref-nodes", false);
dbf.setXIncludeAware(false);
dbf.setExpandEntityReferences(false);
return dbf;
} catch (Exception e) {
throw new RuntimeException("Fail to build secure XML DocumentBuilderFactory", e);
}
}
}
}
@@ -98,6 +98,8 @@ public class RootNode {
*/
private @Nullable JadxDecompiler decompiler;
private @Nullable ManifestAttributes manifestAttributes;
public RootNode(JadxArgs args) {
this.args = args;
this.preDecompilePasses = Jadx.getPreDecompilePassesList();
@@ -211,7 +213,7 @@ public class RootNode {
if (parser != null) {
processResources(parser.getResStorage());
updateObfuscatedFiles(parser, resources);
ManifestAttributes.getInstance().updateAttributes(parser);
initManifestAttributes().updateAttributes(parser);
}
} catch (Exception e) {
LOG.error("Failed to parse 'resources.pb'/'.arsc' file", e);
@@ -721,4 +723,13 @@ public class RootNode {
public GradleInfoStorage getGradleInfoStorage() {
return gradleInfoStorage;
}
public synchronized ManifestAttributes initManifestAttributes() {
ManifestAttributes attrs = manifestAttributes;
if (attrs == null) {
attrs = new ManifestAttributes(args.getSecurity());
manifestAttributes = attrs;
}
return attrs;
}
}
@@ -95,7 +95,8 @@ public class ExportGradleProject {
AppAttribute.MIN_SDK_VERSION,
AppAttribute.TARGET_SDK_VERSION,
AppAttribute.VERSION_CODE,
AppAttribute.VERSION_NAME));
AppAttribute.VERSION_NAME),
root.getArgs().getSecurity());
return parser.parse();
}
}
@@ -495,6 +495,11 @@ public class Utils {
}
}
/**
* @deprecated env vars shouldn't be used in core modules.
* Prefer to parse in `app` (use JadxCommonEnv from 'app-commons') and set in jadx args.
*/
@Deprecated
public static boolean getEnvVarBool(String varName, boolean defValue) {
String strValue = System.getenv(varName);
if (strValue == null) {
@@ -503,6 +508,11 @@ public class Utils {
return strValue.equalsIgnoreCase("true");
}
/**
* @deprecated env vars shouldn't be used in core modules.
* Prefer to parse in `app` (use JadxCommonEnv from 'app-commons') and set in jadx args.
*/
@Deprecated
public static int getEnvVarInt(String varName, int defValue) {
String strValue = System.getenv(varName);
if (strValue == null) {
@@ -1,38 +1,37 @@
package jadx.core.utils.android;
import java.io.StringReader;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.EnumSet;
import java.util.List;
import javax.xml.parsers.DocumentBuilder;
import java.util.Objects;
import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.NodeList;
import org.xml.sax.InputSource;
import jadx.api.ResourceFile;
import jadx.api.ResourceType;
import jadx.api.security.IJadxSecurity;
import jadx.core.utils.exceptions.JadxRuntimeException;
import jadx.core.xmlgen.ResContainer;
import jadx.core.xmlgen.XmlSecurity;
public class AndroidManifestParser {
private static final Logger LOG = LoggerFactory.getLogger(AndroidManifestParser.class);
private final Document androidManifest;
private final Document appStrings;
private final EnumSet<AppAttribute> parseAttrs;
private final IJadxSecurity security;
public AndroidManifestParser(ResourceFile androidManifestRes, EnumSet<AppAttribute> parseAttrs) {
this(androidManifestRes, null, parseAttrs);
public AndroidManifestParser(ResourceFile androidManifestRes, EnumSet<AppAttribute> parseAttrs, IJadxSecurity security) {
this(androidManifestRes, null, parseAttrs, security);
}
public AndroidManifestParser(ResourceFile androidManifestRes, ResContainer appStrings, EnumSet<AppAttribute> parseAttrs) {
public AndroidManifestParser(ResourceFile androidManifestRes, ResContainer appStrings,
EnumSet<AppAttribute> parseAttrs, IJadxSecurity security) {
this.parseAttrs = parseAttrs;
this.security = Objects.requireNonNull(security);
this.androidManifest = parseAndroidManifest(androidManifestRes);
this.appStrings = parseAppStrings(appStrings);
@@ -206,10 +205,9 @@ public class AndroidManifestParser {
return false;
}
private static Document parseXml(String xmlContent) {
try {
DocumentBuilder builder = XmlSecurity.getDBF().newDocumentBuilder();
Document document = builder.parse(new InputSource(new StringReader(xmlContent)));
private Document parseXml(String xmlContent) {
try (InputStream xmlStream = new ByteArrayInputStream(xmlContent.getBytes(StandardCharsets.UTF_8))) {
Document document = security.parseXml(xmlStream);
document.getDocumentElement().normalize();
return document;
} catch (Exception e) {
@@ -217,7 +215,7 @@ public class AndroidManifestParser {
}
}
private static Document parseAppStrings(ResContainer appStrings) {
private Document parseAppStrings(ResContainer appStrings) {
if (appStrings == null) {
return null;
}
@@ -225,7 +223,7 @@ public class AndroidManifestParser {
return parseXml(content);
}
private static Document parseAndroidManifest(ResourceFile androidManifest) {
private Document parseAndroidManifest(ResourceFile androidManifest) {
if (androidManifest == null) {
return null;
}
@@ -23,21 +23,13 @@ import jadx.core.utils.android.AndroidResourcesMap;
import jadx.core.utils.exceptions.JadxRuntimeException;
import jadx.core.xmlgen.entry.ValuesParser;
/*
* TODO:
* Don't die when error occurs
* Check error cases, maybe checked const values are not always the same
* Better error messages
* What to do, when Binary XML Manifest is > size(int)?
* Check for missing chunk size types
* Implement missing data types
* Use line numbers to recreate EXACT AndroidManifest
* Check Element chunk size
*/
public class BinaryXMLParser extends CommonBinaryParser {
private static final Logger LOG = LoggerFactory.getLogger(BinaryXMLParser.class);
private final RootNode rootNode;
private final ManifestAttributes manifestAttributes;
private final boolean attrNewLine;
private final Map<Integer, String> resNames;
private Map<String, String> nsMap;
private Set<String> nsMapGenerated;
@@ -53,16 +45,13 @@ public class BinaryXMLParser extends CommonBinaryParser {
private boolean isOneLine = true;
private int namespaceDepth = 0;
private @Nullable int[] resourceIds;
private final RootNode rootNode;
private String appPackageName;
private Map<String, ClassNode> classNameCache;
private final boolean attrNewLine;
public BinaryXMLParser(RootNode rootNode) {
this.rootNode = rootNode;
this.manifestAttributes = rootNode.initManifestAttributes();
this.attrNewLine = !rootNode.getArgs().isSkipXmlPrettyPrint();
try {
ConstStorage constStorage = rootNode.getConstValues();
@@ -325,7 +314,7 @@ public class BinaryXMLParser extends CommonBinaryParser {
writer.add(' ');
}
writer.add(attrFullName).add("=\"");
String decodedAttr = ManifestAttributes.getInstance().decode(attrName, attrValData);
String decodedAttr = manifestAttributes.decode(attrName, attrValData);
if (decodedAttr != null) {
memorizePackageName(attrName, decodedAttr);
if (isDeobfCandidateAttr(attrFullName)) {
@@ -419,8 +408,7 @@ public class BinaryXMLParser extends CommonBinaryParser {
return "NOT_FOUND_STR_0x" + Integer.toHexString(strId);
}
private void decodeAttribute(int attributeNS, int attrValDataType, int attrValData,
String attrFullName) {
private void decodeAttribute(int attributeNS, int attrValDataType, int attrValData, String attrFullName) {
if (attrValDataType == TYPE_REFERENCE) {
// reference custom processing
String resName = resNames.get(attrValData);
@@ -7,8 +7,6 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import javax.xml.parsers.DocumentBuilder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.w3c.dom.Document;
@@ -16,11 +14,17 @@ import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import jadx.api.security.IJadxSecurity;
import jadx.core.utils.exceptions.JadxRuntimeException;
import jadx.core.xmlgen.entry.RawNamedValue;
import jadx.core.xmlgen.entry.ResourceEntry;
import jadx.core.xmlgen.entry.ValuesParser;
// TODO: move to Android specific module!
/**
* Load and store Android Manifest attributes specification
*/
public class ManifestAttributes {
private static final Logger LOG = LoggerFactory.getLogger(ManifestAttributes.class);
@@ -53,24 +57,12 @@ public class ManifestAttributes {
}
}
private final IJadxSecurity security;
private final Map<String, MAttr> attrMap = new HashMap<>();
private final Map<String, MAttr> appAttrMap = new HashMap<>();
private static ManifestAttributes instance;
public static ManifestAttributes getInstance() {
if (instance == null) {
try {
instance = new ManifestAttributes();
} catch (Exception e) {
LOG.error("Failed to create ManifestAttributes", e);
}
}
return instance;
}
private ManifestAttributes() {
public ManifestAttributes(IJadxSecurity security) {
this.security = security;
parseAll();
}
@@ -86,8 +78,7 @@ public class ManifestAttributes {
if (xmlStream == null) {
throw new JadxRuntimeException(xml + " not found in classpath");
}
DocumentBuilder dBuilder = XmlSecurity.getDBF().newDocumentBuilder();
doc = dBuilder.parse(xmlStream);
doc = security.parseXml(xmlStream);
} catch (Exception e) {
throw new JadxRuntimeException("Xml load error, file: " + xml, e);
}
@@ -87,7 +87,7 @@ public class ResTableBinaryParser extends CommonBinaryParser implements IResTabl
public void decode(InputStream inputStream) throws IOException {
long start = System.currentTimeMillis();
is = new ParserStream(inputStream);
resStorage = new ResourceStorage();
resStorage = new ResourceStorage(root.getArgs().getSecurity());
decodeTableChunk();
resStorage.finish();
if (LOG.isDebugEnabled()) {
@@ -99,7 +99,7 @@ public class ResTableBinaryParser extends CommonBinaryParser implements IResTabl
@Override
public ResContainer decodeFiles() {
ValuesParser vp = new ValuesParser(strings, resStorage.getResourcesNames());
ResXmlGen resGen = new ResXmlGen(resStorage, vp);
ResXmlGen resGen = new ResXmlGen(resStorage, vp, root.initManifestAttributes());
ICodeInfo content = XmlGenUtils.makeXmlDump(root.makeCodeWriter(), resStorage);
List<ResContainer> xmlFiles = resGen.makeResourcesXml(root.getArgs());
@@ -43,10 +43,12 @@ public class ResXmlGen {
private final ResourceStorage resStorage;
private final ValuesParser vp;
private final ManifestAttributes manifestAttributes;
public ResXmlGen(ResourceStorage resStorage, ValuesParser vp) {
public ResXmlGen(ResourceStorage resStorage, ValuesParser vp, ManifestAttributes manifestAttributes) {
this.resStorage = resStorage;
this.vp = vp;
this.manifestAttributes = manifestAttributes;
}
public List<ResContainer> makeResourcesXml(JadxArgs args) {
@@ -191,7 +193,7 @@ public class ResXmlGen {
if (dataType == ParserConstants.TYPE_INT_DEC && nameStr != null) {
try {
int intVal = Integer.parseInt(valueStr);
String newVal = ManifestAttributes.getInstance().decode(nameStr.replace("android:", "").replace("attr.", ""), intVal);
String newVal = manifestAttributes.decode(nameStr.replace("android:", "").replace("attr.", ""), intVal);
if (newVal != null) {
valueStr = newVal;
}
@@ -202,7 +204,7 @@ public class ResXmlGen {
if (dataType == ParserConstants.TYPE_INT_HEX && nameStr != null) {
try {
int intVal = Integer.decode(valueStr);
String newVal = ManifestAttributes.getInstance().decode(nameStr.replace("android:", "").replace("attr.", ""), intVal);
String newVal = manifestAttributes.decode(nameStr.replace("android:", "").replace("attr.", ""), intVal);
if (newVal != null) {
valueStr = newVal;
}
@@ -7,6 +7,7 @@ import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import jadx.api.security.IJadxSecurity;
import jadx.core.xmlgen.entry.ResourceEntry;
public class ResourceStorage {
@@ -16,6 +17,8 @@ public class ResourceStorage {
.thenComparing(ResourceEntry::getKeyName);
private final List<ResourceEntry> list = new ArrayList<>();
private final IJadxSecurity security;
private String appPackage;
/**
@@ -28,6 +31,10 @@ public class ResourceStorage {
*/
private final Map<Integer, String> renames = new HashMap<>();
public ResourceStorage(IJadxSecurity security) {
this.security = security;
}
public void add(ResourceEntry resEntry) {
list.add(resEntry);
uniqNameEntries.put(resEntry, resEntry);
@@ -76,7 +83,7 @@ public class ResourceStorage {
}
public void setAppPackage(String appPackage) {
this.appPackage = XmlSecurity.verifyAppPackage(appPackage);
this.appPackage = security.verifyAppPackage(appPackage);
}
public Map<Integer, String> getResourcesNames() {
@@ -1,54 +0,0 @@
package jadx.core.xmlgen;
import javax.xml.parsers.DocumentBuilderFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import jadx.core.deobf.NameMapper;
import jadx.core.utils.Utils;
public class XmlSecurity {
private static final Logger LOG = LoggerFactory.getLogger(XmlSecurity.class);
private static final boolean DISABLE_CHECKS = Utils.getEnvVarBool("JADX_DISABLE_XML_SECURITY", false);
private static final DocumentBuilderFactory DBF_INSTANCE = buildDBF();
private XmlSecurity() {
}
public static DocumentBuilderFactory getDBF() {
return DBF_INSTANCE;
}
public static String verifyAppPackage(String appPackage) {
if (DISABLE_CHECKS) {
return appPackage;
}
if (NameMapper.isValidFullIdentifier(appPackage)) {
return appPackage;
}
LOG.warn("App package '{}' has invalid format and will be ignored", appPackage);
return "INVALID_PACKAGE";
}
private static DocumentBuilderFactory buildDBF() {
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
if (DISABLE_CHECKS) {
return dbf;
}
try {
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
dbf.setFeature("http://apache.org/xml/features/dom/create-entity-ref-nodes", false);
dbf.setXIncludeAware(false);
dbf.setExpandEntityReferences(false);
return dbf;
} catch (Exception e) {
throw new RuntimeException("Fail to build secure XML DocumentBuilderFactory", e);
}
}
}