From 8740a14957238cbb09555d7877729a669b89b036 Mon Sep 17 00:00:00 2001 From: ix0rai Date: Thu, 7 Nov 2024 16:56:53 -0600 Subject: [PATCH] do not fully validate parameter indices in hasEntry (#238) * do not fully validate parameter indices in hasEntry * checkstyle --- .../org/quiltmc/enigma/api/EnigmaProject.java | 42 ++++++++++++- .../api/analysis/index/jar/EntryIndex.java | 59 +++++++------------ .../translation/mapping/MappingsChecker.java | 4 +- 3 files changed, 63 insertions(+), 42 deletions(-) diff --git a/enigma/src/main/java/org/quiltmc/enigma/api/EnigmaProject.java b/enigma/src/main/java/org/quiltmc/enigma/api/EnigmaProject.java index 67e7b498..a304cff8 100644 --- a/enigma/src/main/java/org/quiltmc/enigma/api/EnigmaProject.java +++ b/enigma/src/main/java/org/quiltmc/enigma/api/EnigmaProject.java @@ -170,7 +170,7 @@ public boolean isNavigable(Entry obfEntry) { return false; } - return this.jarIndex.getIndex(EntryIndex.class).hasEntry(obfEntry, this); + return this.jarIndex.getIndex(EntryIndex.class).hasEntry(obfEntry); } public boolean isRenamable(Entry obfEntry) { @@ -211,7 +211,7 @@ public boolean isRenamable(Entry obfEntry) { return false; } - return this.jarIndex.getIndex(EntryIndex.class).hasEntry(obfEntry, this); + return this.jarIndex.getIndex(EntryIndex.class).hasEntry(obfEntry); } private static boolean isEnumValueOfMethod(ClassDefEntry parent, MethodEntry method) { @@ -237,7 +237,7 @@ public boolean isObfuscated(Entry entry) { } public boolean isSynthetic(Entry entry) { - return this.jarIndex.getIndex(EntryIndex.class).hasEntry(entry, this) && this.jarIndex.getIndex(EntryIndex.class).getEntryAccess(entry).isSynthetic(); + return this.jarIndex.getIndex(EntryIndex.class).hasEntry(entry) && this.jarIndex.getIndex(EntryIndex.class).getEntryAccess(entry).isSynthetic(); } public boolean isAnonymousOrLocal(ClassEntry classEntry) { @@ -246,6 +246,42 @@ public boolean isAnonymousOrLocal(ClassEntry classEntry) { return enclosingMethodIndex.hasEnclosingMethod(classEntry); } + /** + * Verifies that the provided {@code parameter} has a valid index for its parent method. + * This method validates both the upper and lower bounds of the parent method's index range. + * + *

Note that this method could still return {@code true} for an invalid index in the case that the index is impossible due to double-size parameters -- + * for example, if the index is 4 and there's a double at index 3, the index would be invalid. + * But honestly, we at QuiltMC doubt that that's a situation you'll ever be running into. + * If it is, complain in our issue tracker about us writing this whole comment instead of implementing that functionality. + * + * @param parameter the parameter to validate + * @return whether the index is valid + */ + public boolean validateParameterIndex(LocalVariableEntry parameter) { + MethodEntry parent = parameter.getParent(); + EntryIndex index = this.jarIndex.getIndex(EntryIndex.class); + + if (index.hasMethod(parent)) { + AtomicInteger maxLocals = new AtomicInteger(-1); + ClassEntry parentClass = parent != null ? parent.getParent() : null; + + // find max_locals for method, representing the number of parameters it receives (JVMS§4.7.3) + // note: parent class cannot be null, warning suppressed + ClassNode classNode = this.getClassProvider().get(parentClass.getFullName()); + if (classNode != null) { + classNode.methods.stream() + .filter(node -> node.name.equals(parent.getName()) && node.desc.equals(parent.getDesc().toString())) + .findFirst().ifPresent(node -> maxLocals.set(node.maxLocals)); + } + + // if maxLocals is -1 it's not found for the method and should be ignored + return index.validateParameterIndex(parameter) && (maxLocals.get() == -1 || parameter.getIndex() <= maxLocals.get() - 1); + } + + return false; + } + public JarExport exportRemappedJar(ProgressListener progress) { Collection classEntries = this.jarIndex.getIndex(EntryIndex.class).getClasses(); ClassProvider fixingClassProvider = new ObfuscationFixClassProvider(this.classProvider, this.jarIndex); diff --git a/enigma/src/main/java/org/quiltmc/enigma/api/analysis/index/jar/EntryIndex.java b/enigma/src/main/java/org/quiltmc/enigma/api/analysis/index/jar/EntryIndex.java index 817507ce..4a3abe7a 100644 --- a/enigma/src/main/java/org/quiltmc/enigma/api/analysis/index/jar/EntryIndex.java +++ b/enigma/src/main/java/org/quiltmc/enigma/api/analysis/index/jar/EntryIndex.java @@ -1,6 +1,5 @@ package org.quiltmc.enigma.api.analysis.index.jar; -import org.objectweb.asm.tree.ClassNode; import org.quiltmc.enigma.api.EnigmaProject; import org.quiltmc.enigma.api.translation.mapping.EntryMapping; import org.quiltmc.enigma.api.translation.mapping.tree.EntryTree; @@ -19,7 +18,6 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.atomic.AtomicInteger; public class EntryIndex implements JarIndexer { private final EntryTree tree = new HashEntryTree<>(); @@ -73,26 +71,13 @@ public boolean hasField(FieldEntry entry) { /** * Checks whether the entry has been indexed and therefore exists in the JAR file. *
- * Parameters are not indexed, and this method does not fully verify validity of parameter indices. - * Therefore, it is possible that this method returns {@code true} for an invalid parameter. + * For parameters, which are not indexed, checks the parent method and partially verifies their indices. + * To fully validate a parameter's index, use {@link EnigmaProject#validateParameterIndex(LocalVariableEntry)}. + * * @param entry the entry to check * @return whether the entry exists - * @see #hasEntry(Entry, EnigmaProject) */ public boolean hasEntry(Entry entry) { - return this.hasEntry(entry, null); - } - - /** - * Checks whether the entry has been indexed and therefore exists in the JAR file. - *
- * For parameters, which are not indexed, verifies that they have a valid index and therefore could exist. - * @param entry the entry to check - * @param project the current project - * @return whether the entry exists - */ - @SuppressWarnings("ConstantConditions") - public boolean hasEntry(Entry entry, @Nullable EnigmaProject project) { if (entry instanceof ClassEntry classEntry) { return this.hasClass(classEntry); } else if (entry instanceof MethodEntry methodEntry) { @@ -101,32 +86,30 @@ public boolean hasEntry(Entry entry, @Nullable EnigmaProject project) { return this.hasField(fieldEntry); } else if (entry instanceof LocalVariableEntry localVariableEntry) { MethodEntry parent = localVariableEntry.getParent(); - if (this.hasMethod(parent)) { - AtomicInteger maxLocals = new AtomicInteger(-1); - ClassEntry parentClass = parent != null ? parent.getParent() : null; - - if (project != null) { - // find max_locals for method, representing the number of parameters it receives (JVMS§4.7.3) - // note: parent class cannot be null, warning suppressed - ClassNode classNode = project.getClassProvider().get(parentClass.getFullName()); - if (classNode != null) { - classNode.methods.stream() - .filter(node -> node.name.equals(parent.getName()) && node.desc.equals(parent.getDesc().toString())) - .findFirst().ifPresent(node -> maxLocals.set(node.maxLocals)); - } - } - - AccessFlags parentAccess = this.getMethodAccess(parent); - int startIndex = parentAccess != null && parentAccess.isStatic() ? 0 : 1; - - // if maxLocals is -1 it's not found for the method and should be ignored - return localVariableEntry.getIndex() >= startIndex && (maxLocals.get() == -1 || localVariableEntry.getIndex() <= maxLocals.get() - 1); + if (this.hasEntry(parent)) { + return this.validateParameterIndex(localVariableEntry); } } return false; } + /** + * Validates that the parameter index is not below the minimum index for its parent method and therefore could be valid. + *
Note that this method does not guarantee the index is valid -- for full validation, call {@link EnigmaProject#validateParameterIndex(LocalVariableEntry)}. + * + * @param parameter the parameter to validate + * @return whether the index could be valid + * @see EnigmaProject#validateParameterIndex(LocalVariableEntry) + */ + public boolean validateParameterIndex(LocalVariableEntry parameter) { + MethodEntry parent = parameter.getParent(); + AccessFlags parentAccess = this.getMethodAccess(parent); + + int startIndex = parentAccess != null && parentAccess.isStatic() ? 0 : 1; + return parameter.getIndex() >= startIndex; + } + @Nullable public AccessFlags getMethodAccess(MethodEntry entry) { var def = this.methodDefinitions.get(entry); diff --git a/enigma/src/main/java/org/quiltmc/enigma/impl/translation/mapping/MappingsChecker.java b/enigma/src/main/java/org/quiltmc/enigma/impl/translation/mapping/MappingsChecker.java index c1e9d189..89b0fab1 100644 --- a/enigma/src/main/java/org/quiltmc/enigma/impl/translation/mapping/MappingsChecker.java +++ b/enigma/src/main/java/org/quiltmc/enigma/impl/translation/mapping/MappingsChecker.java @@ -9,6 +9,7 @@ import org.quiltmc.enigma.api.translation.mapping.tree.EntryTree; import org.quiltmc.enigma.api.translation.mapping.tree.EntryTreeNode; import org.quiltmc.enigma.api.translation.representation.entry.Entry; +import org.quiltmc.enigma.api.translation.representation.entry.LocalVariableEntry; import org.quiltmc.enigma.api.translation.representation.entry.MethodEntry; import java.util.Collection; @@ -62,7 +63,8 @@ private void tryDropBrokenEntry(Dropped dropped, Entry entry) { } private boolean shouldDropBrokenEntry(Dropped dropped, Entry entry) { - if (!this.index.getIndex(EntryIndex.class).hasEntry(entry, this.project)) { + if (!this.index.getIndex(EntryIndex.class).hasEntry(entry) + || (entry instanceof LocalVariableEntry parameter && !this.project.validateParameterIndex(parameter))) { return true; }