From 11e40cee6e397755dc20eca7bdd1ecd80279e637 Mon Sep 17 00:00:00 2001
From: Eduardo Speroni <edusperoni@users.noreply.github.com>
Date: Mon, 2 Sep 2024 16:44:55 -0300
Subject: [PATCH 1/2] fix: generate correct metadata when overflowing signed
short values (#1821)
---
.../src/com/telerik/metadata/Generator.java | 11 +++++-
.../src/src/com/telerik/metadata/Writer.java | 34 ++++++++++++++++++-
.../runtime/src/main/cpp/MetadataReader.cpp | 17 +++++++---
3 files changed, 56 insertions(+), 6 deletions(-)
diff --git a/test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/Generator.java b/test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/Generator.java
index 5e11277a4..6a53cea27 100644
--- a/test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/Generator.java
+++ b/test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/Generator.java
@@ -27,6 +27,8 @@ public class Generator {
private static final String MDG_BLACKLIST = "blacklist.mdg";
private static final String METADATA_JAVA_OUT = "mdg-java-out.txt";
+ private static boolean verbose_mode = false;
+
/**
* @param args arguments
*/
@@ -69,7 +71,13 @@ public static void main(String[] args) {
FileOutputStream oss = new FileOutputStream(new File(metadataOutputDir, "treeStringsStream.dat"));
FileStreamWriter outStringsStream = new FileStreamWriter(oss);
- new Writer(outNodeStream, outValueStream, outStringsStream).writeTree(root);
+ if (verbose_mode) {
+ FileOutputStream ods = new FileOutputStream(new File("metadata-debug.json"));
+ FileStreamWriter outDebugStream = new FileStreamWriter(ods);
+ new Writer(outNodeStream, outValueStream, outStringsStream, outDebugStream).writeTree(root);
+ } else {
+ new Writer(outNodeStream, outValueStream, outStringsStream).writeTree(root);
+ }
} catch (Throwable ex) {
System.err.println(String.format("Error executing Metadata Generator: %s", ex.getMessage()));
ex.printStackTrace(System.out);
@@ -83,6 +91,7 @@ private static void enableFlaggedFeatures(String[] args) {
String filePath = arg.replace(ANALYTICS_ARGUMENT_BEGINNING, "");
AnalyticsConfiguration.enableAnalytics(filePath);
} else if (VERBOSE_FLAG_NAME.equals(arg)) {
+ verbose_mode = true;
MetadataFilterConsoleLogger.INSTANCE.setEnabled(true);
} else if (SKIP_FLAG_NAME.equals(arg)) {
System.out.println("Skipping metadata generation: skip flag used.");
diff --git a/test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/Writer.java b/test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/Writer.java
index 1d76f2550..36777a831 100644
--- a/test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/Writer.java
+++ b/test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/Writer.java
@@ -1,5 +1,7 @@
package com.telerik.metadata;
+import com.google.gson.JsonArray;
+import com.google.gson.JsonObject;
import com.telerik.metadata.TreeNode.FieldInfo;
import com.telerik.metadata.TreeNode.MethodInfo;
@@ -10,6 +12,7 @@
import java.nio.charset.StandardCharsets;
import java.util.ArrayDeque;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Optional;
@@ -18,14 +21,20 @@ public class Writer {
private final StreamWriter outNodeStream;
private final StreamWriter outValueStream;
private final StreamWriter outStringsStream;
+ private final StreamWriter outDebugStream;
private int commonInterfacePrefixPosition;
public Writer(StreamWriter outNodeStream, StreamWriter outValueStream,
StreamWriter outStringsStream) {
+ this(outNodeStream, outValueStream, outStringsStream, null);
+ }
+ public Writer(StreamWriter outNodeStream, StreamWriter outValueStream,
+ StreamWriter outStringsStream, StreamWriter outDebugStream) {
this.outNodeStream = outNodeStream;
this.outValueStream = outValueStream;
this.outStringsStream = outStringsStream;
+ this.outDebugStream = outDebugStream;
}
private final static byte[] writeUniqueName_lenBuff = new byte[2];
@@ -214,6 +223,10 @@ public void writeTree(TreeNode root) throws Exception {
d.push(root);
while (!d.isEmpty()) {
TreeNode n = d.pollFirst();
+ if (Short.toUnsignedInt((short)(curId + 1)) < Short.toUnsignedInt(curId)) {
+ // we have overflowed our maximum (16 bit) metadata size
+ throw new Exception("Metadata is too big and has overflown our current limit, please report this issue");
+ }
n.id = n.firstChildId = n.nextSiblingId = curId++;
String name = n.getName();
@@ -351,7 +364,7 @@ public void writeTree(TreeNode root) throws Exception {
while (!d.isEmpty()) {
TreeNode n = d.pollFirst();
- nodeData[0] = n.firstChildId + (n.nextSiblingId << 16);
+ nodeData[0] = (n.firstChildId & 0xFFFF) | (n.nextSiblingId << 16);
nodeData[1] = n.offsetName;
nodeData[2] = n.offsetValue;
@@ -364,5 +377,24 @@ public void writeTree(TreeNode root) throws Exception {
outNodeStream.flush();
outNodeStream.close();
+
+ if (outDebugStream != null) {
+ d.push(root);
+ JsonArray rootArray = new JsonArray();
+ while (!d.isEmpty()) {
+ TreeNode n = d.pollFirst();
+ JsonObject obj = new JsonObject();
+ obj.addProperty("id", Short.toUnsignedInt(n.id));
+ obj.addProperty("nextSiblingId", Short.toUnsignedInt(n.nextSiblingId));
+ obj.addProperty("firstChildId", Short.toUnsignedInt(n.firstChildId));
+ obj.addProperty("name", n.getName());
+ obj.addProperty("nodeType", n.nodeType);
+ rootArray.add(obj);
+ d.addAll(n.children);
+ }
+ outDebugStream.write(rootArray.toString().getBytes());
+ outDebugStream.flush();
+ outDebugStream.close();
+ }
}
}
diff --git a/test-app/runtime/src/main/cpp/MetadataReader.cpp b/test-app/runtime/src/main/cpp/MetadataReader.cpp
index 9fb9daa78..5c37dfdcb 100644
--- a/test-app/runtime/src/main/cpp/MetadataReader.cpp
+++ b/test-app/runtime/src/main/cpp/MetadataReader.cpp
@@ -2,6 +2,7 @@
#include "MetadataMethodInfo.h"
#include "Util.h"
#include <sstream>
+#include <android/log.h>
using namespace std;
using namespace tns;
@@ -54,20 +55,28 @@ MetadataTreeNode* MetadataReader::BuildTree() {
while (true) {
uint16_t childNodeDataId = childNodeData - rootNodeData;
+
+ MetadataTreeNode* childNode;
// node (and its next siblings) already visited, so we don't need to visit it again
if (m_v[childNodeDataId] != emptyNode) {
+ childNode = m_v[childNodeDataId];
+ __android_log_print(ANDROID_LOG_ERROR, "TNS.error", "Consistency error in metadata. A child should never have been visited before its parent. Parent: %s Child: %s. Child metadata id: %u", node->name.c_str(), childNode->name.c_str(), childNodeDataId);
break;
+ } else {
+ childNode = new MetadataTreeNode;
+ childNode->name = ReadName(childNodeData->offsetName);
+ childNode->offsetValue = childNodeData->offsetValue;
}
-
- MetadataTreeNode* childNode = new MetadataTreeNode;
childNode->parent = node;
- childNode->name = ReadName(childNodeData->offsetName);
- childNode->offsetValue = childNodeData->offsetValue;
node->children->push_back(childNode);
m_v[childNodeDataId] = childNode;
+ if (childNodeDataId == childNodeData->nextSiblingId) {
+ break;
+ }
+
childNodeData = rootNodeData + childNodeData->nextSiblingId;
}
}
From 1a73f6ffbb4d544d4c1da58daf9068c702985d35 Mon Sep 17 00:00:00 2001
From: Eduardo Speroni <edusperoni@gmail.com>
Date: Mon, 30 Sep 2024 10:53:22 -0300
Subject: [PATCH 2/2] fix: prevent metadata offset overflow into array space
and convert shorts to uints before addition
---
.../src/src/com/telerik/metadata/Writer.java | 10 ++++++++--
test-app/runtime/src/main/cpp/MetadataReader.h | 2 +-
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/Writer.java b/test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/Writer.java
index 36777a831..a09472fa2 100644
--- a/test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/Writer.java
+++ b/test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/Writer.java
@@ -305,7 +305,7 @@ public void writeTree(TreeNode root) throws Exception {
outStringsStream.close();
writeInt(0, outValueStream);
- final int array_offset = 1000 * 1000 * 1000;
+ final int array_offset = Integer.MAX_VALUE; // 2147483647, which is half of uint32
d.push(root);
while (!d.isEmpty()) {
@@ -328,6 +328,10 @@ public void writeTree(TreeNode root) throws Exception {
throw new Exception("should not happen");
}
+ if ((n.nodeType & TreeNode.Array) != TreeNode.Array && Integer.toUnsignedLong(n.offsetValue) >= Integer.toUnsignedLong(array_offset)) {
+ throw new Exception("Non-array metadata has overflown array space. Please report this issue.");
+ }
+
d.addAll(n.children);
}
@@ -339,7 +343,7 @@ public void writeTree(TreeNode root) throws Exception {
TreeNode n = d.pollFirst();
if (n.arrayElement != null) {
- n.offsetValue = array_offset + n.arrayElement.id;
+ n.offsetValue = array_offset + Short.toUnsignedInt(n.arrayElement.id);
}
if (!n.children.isEmpty()) {
@@ -387,6 +391,8 @@ public void writeTree(TreeNode root) throws Exception {
obj.addProperty("id", Short.toUnsignedInt(n.id));
obj.addProperty("nextSiblingId", Short.toUnsignedInt(n.nextSiblingId));
obj.addProperty("firstChildId", Short.toUnsignedInt(n.firstChildId));
+ obj.addProperty("offsetName", Integer.toUnsignedLong(n.offsetName));
+ obj.addProperty("offsetValue", Integer.toUnsignedLong(n.offsetValue));
obj.addProperty("name", n.getName());
obj.addProperty("nodeType", n.nodeType);
rootArray.add(obj);
diff --git a/test-app/runtime/src/main/cpp/MetadataReader.h b/test-app/runtime/src/main/cpp/MetadataReader.h
index 1f3706994..22f7fbfdd 100644
--- a/test-app/runtime/src/main/cpp/MetadataReader.h
+++ b/test-app/runtime/src/main/cpp/MetadataReader.h
@@ -167,7 +167,7 @@ class MetadataReader {
private:
- static const uint32_t ARRAY_OFFSET = 1000000000;
+ static const uint32_t ARRAY_OFFSET = INT32_MAX; // 2147483647
MetadataTreeNode* BuildTree();