Merge pull request #7000 from alvasw/txvalidator_fix_crash_on_invalid…

…_vin_vout_json_array TxValidator: Fix crash on invalid vin or vout JSON Array
bisq-network · Jan 14, 2024 · 4d4c9e3 · 4d4c9e3
2 parents 5872e9e + d7cd1e2
commit 4d4c9e3
Show file tree

Hide file tree

Showing 3 changed files with 185 additions and 12 deletions.
diff --git a/core/src/main/java/bisq/core/provider/mempool/TxValidator.java b/core/src/main/java/bisq/core/provider/mempool/TxValidator.java
@@ -302,12 +302,17 @@ private static Tuple2<JsonArray, JsonArray> getVinAndVout(String jsonTxt) throws
         if (json.get("vin") == null || json.get("vout") == null) {
             throw new JsonSyntaxException("missing vin/vout");
         }
-        JsonArray jsonVin = json.get("vin").getAsJsonArray();
-        JsonArray jsonVout = json.get("vout").getAsJsonArray();
-        if (jsonVin == null || jsonVout == null || jsonVin.size() < 1 || jsonVout.size() < 2) {
-            throw new JsonSyntaxException("not enough vins/vouts");
+
+        try {
+            JsonArray jsonVin = json.get("vin").getAsJsonArray();
+            JsonArray jsonVout = json.get("vout").getAsJsonArray();
+            if (jsonVin == null || jsonVout == null || jsonVin.size() < 1 || jsonVout.size() < 2) {
+                throw new JsonSyntaxException("not enough vins/vouts");
+            }
+            return new Tuple2<>(jsonVin, jsonVout);
+        } catch (IllegalStateException e) {
+            throw new JsonSyntaxException("vin/vout no as JSON Array", e);
         }
-        return new Tuple2<>(jsonVin, jsonVout);
     }
 
     private static FeeValidationStatus initialSanityChecks(String txId, String jsonTxt) {

diff --git a/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java b/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java
@@ -0,0 +1,153 @@
+/*
+ * This file is part of Bisq.
+ *
+ * Bisq is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or (at
+ * your option) any later version.
+ *
+ * Bisq is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Bisq. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package bisq.core.fee;
+
+import bisq.core.dao.state.DaoStateService;
+import bisq.core.filter.FilterManager;
+import bisq.core.provider.mempool.FeeValidationStatus;
+import bisq.core.provider.mempool.TxValidator;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonPrimitive;
+
+import java.net.URL;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import java.io.IOException;
+
+import java.util.List;
+import java.util.Objects;
+
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.NullAndEmptySource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+@ExtendWith(MockitoExtension.class)
+public class MakerTxValidatorSanityCheckTests {
+    public static final List<String> FEE_RECEIVER_ADDRESSES = List.of("2MzBNTJDjjXgViKBGnatDU3yWkJ8pJkEg9w");
+
+    private TxValidator txValidator;
+
+    @BeforeEach
+    void setup(@Mock DaoStateService daoStateService, @Mock FilterManager filterManager) {
+        String txId = "e3607e971ead7d03619e3a9eeaa771ed5adba14c448839e0299f857f7bb4ec07";
+        txValidator = new TxValidator(daoStateService, txId, filterManager);
+    }
+
+    @ParameterizedTest
+    @NullAndEmptySource
+    void nullAndEmptyMempoolResponse(String jsonText) {
+        TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonText, FEE_RECEIVER_ADDRESSES);
+        FeeValidationStatus status = txValidator1.getStatus();
+        assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR)));
+    }
+
+    @Test
+    void invalidJsonResponse() {
+        String invalidJson = "in\"valid'json',";
+        TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(invalidJson, FEE_RECEIVER_ADDRESSES);
+
+        FeeValidationStatus status = txValidator1.getStatus();
+        assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR)));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"status", "txid", "vin", "vout"})
+    void mempoolResponseWithMissingField(String missingField) throws IOException {
+        JsonObject json = getValidBtcMakerFeeMempoolJsonResponse();
+        json.remove(missingField);
+        assertThat(json.has(missingField), is(false));
+
+        String jsonContent = new Gson().toJson(json);
+        TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES);
+
+        FeeValidationStatus status = txValidator1.getStatus();
+        assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR)));
+    }
+
+    @Test
+    void mempoolResponseWithoutConfirmedField() throws IOException {
+        JsonObject json = getValidBtcMakerFeeMempoolJsonResponse();
+        json.get("status").getAsJsonObject().remove("confirmed");
+        assertThat(json.get("status").getAsJsonObject().has("confirmed"), is(false));
+
+        String jsonContent = new Gson().toJson(json);
+        TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES);
+
+        FeeValidationStatus status = txValidator1.getStatus();
+        assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR)));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"vin", "vout"})
+    void checkFeeAddressBtcTestVinOrVoutNotJsonArray(String vinOrVout) throws IOException {
+        JsonObject json = MakerTxValidatorSanityCheckTests.getValidBtcMakerFeeMempoolJsonResponse();
+        json.add(vinOrVout, new JsonPrimitive(1234));
+        assertThrows(IllegalStateException.class, () -> json.get(vinOrVout).getAsJsonArray());
+
+        String jsonContent = new Gson().toJson(json);
+        TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent,
+                MakerTxValidatorSanityCheckTests.FEE_RECEIVER_ADDRESSES);
+
+        assertThat(txValidator1.getStatus(), is(FeeValidationStatus.NACK_JSON_ERROR));
+    }
+
+    @Test
+    void responseHasDifferentTxId() throws IOException {
+        String differentTxId = "abcde971ead7d03619e3a9eeaa771ed5adba14c448839e0299f857f7bb4ec07";
+
+        JsonObject json = getValidBtcMakerFeeMempoolJsonResponse();
+        json.add("txid", new JsonPrimitive(differentTxId));
+        assertThat(json.get("txid").getAsString(), is(differentTxId));
+
+        String jsonContent = new Gson().toJson(json);
+        TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES);
+
+        FeeValidationStatus status = txValidator1.getStatus();
+        assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR)));
+    }
+
+    public static JsonObject getValidBtcMakerFeeMempoolJsonResponse() throws IOException {
+        URL resource = MakerTxValidatorSanityCheckTests.class.getClassLoader()
+                .getResource("mempool_test_data/valid_btc_maker_fee.json");
+        String path = Objects.requireNonNull(resource).getPath();
+
+        if (System.getProperty("os.name").toLowerCase().startsWith("win")) {
+            // We need to remove the first character on Windows because the path starts with a
+            // leading slash "/C:/Users/..."
+            path = path.substring(1);
+        }
+
+        String jsonContent = Files.readString(Path.of(path));
+        return new Gson().fromJson(jsonContent, JsonObject.class);
+    }
+}
diff --git a/...core/fee/TxValidatorSanityCheckTests.java → ...fee/TakerTxValidatorSanityCheckTests.java b/...core/fee/TxValidatorSanityCheckTests.java → ...fee/TakerTxValidatorSanityCheckTests.java
@@ -49,9 +49,10 @@
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 @ExtendWith(MockitoExtension.class)
-public class TxValidatorSanityCheckTests {
+public class TakerTxValidatorSanityCheckTests {
     private final List<String> FEE_RECEIVER_ADDRESSES = List.of("2MzBNTJDjjXgViKBGnatDU3yWkJ8pJkEg9w");
 
     private TxValidator txValidator;
@@ -65,29 +66,29 @@ void setup(@Mock DaoStateService daoStateService, @Mock FilterManager filterMana
     @ParameterizedTest
     @NullAndEmptySource
     void nullAndEmptyMempoolResponse(String jsonText) {
-        TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonText, FEE_RECEIVER_ADDRESSES);
+        TxValidator txValidator1 = txValidator.parseJsonValidateTakerFeeTx(jsonText, FEE_RECEIVER_ADDRESSES);
         FeeValidationStatus status = txValidator1.getStatus();
         assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR)));
     }
 
     @Test
     void invalidJsonResponse() {
         String invalidJson = "in\"valid'json',";
-        TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(invalidJson, FEE_RECEIVER_ADDRESSES);
+        TxValidator txValidator1 = txValidator.parseJsonValidateTakerFeeTx(invalidJson, FEE_RECEIVER_ADDRESSES);
 
         FeeValidationStatus status = txValidator1.getStatus();
         assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR)));
     }
 
     @ParameterizedTest
-    @ValueSource(strings = {"status", "txid"})
+    @ValueSource(strings = {"status", "txid", "vin", "vout"})
     void mempoolResponseWithMissingField(String missingField) throws IOException {
         JsonObject json = getValidBtcMakerFeeMempoolJsonResponse();
         json.remove(missingField);
         assertThat(json.has(missingField), is(false));
 
         String jsonContent = new Gson().toJson(json);
-        TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES);
+        TxValidator txValidator1 = txValidator.parseJsonValidateTakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES);
 
         FeeValidationStatus status = txValidator1.getStatus();
         assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR)));
@@ -100,12 +101,26 @@ void mempoolResponseWithoutConfirmedField() throws IOException {
         assertThat(json.get("status").getAsJsonObject().has("confirmed"), is(false));
 
         String jsonContent = new Gson().toJson(json);
-        TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES);
+        TxValidator txValidator1 = txValidator.parseJsonValidateTakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES);
 
         FeeValidationStatus status = txValidator1.getStatus();
         assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR)));
     }
 
+    @ParameterizedTest
+    @ValueSource(strings = {"vin", "vout"})
+    void checkFeeAddressBtcTestVinOrVoutNotJsonArray(String vinOrVout) throws IOException {
+        JsonObject json = MakerTxValidatorSanityCheckTests.getValidBtcMakerFeeMempoolJsonResponse();
+        json.add(vinOrVout, new JsonPrimitive(1234));
+        assertThrows(IllegalStateException.class, () -> json.get(vinOrVout).getAsJsonArray());
+
+        String jsonContent = new Gson().toJson(json);
+        TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent,
+                MakerTxValidatorSanityCheckTests.FEE_RECEIVER_ADDRESSES);
+
+        assertThat(txValidator1.getStatus(), is(FeeValidationStatus.NACK_JSON_ERROR));
+    }
+
     @Test
     void responseHasDifferentTxId() throws IOException {
         String differentTxId = "abcde971ead7d03619e3a9eeaa771ed5adba14c448839e0299f857f7bb4ec07";
@@ -115,7 +130,7 @@ void responseHasDifferentTxId() throws IOException {
         assertThat(json.get("txid").getAsString(), is(differentTxId));
 
         String jsonContent = new Gson().toJson(json);
-        TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES);
+        TxValidator txValidator1 = txValidator.parseJsonValidateTakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES);
 
         FeeValidationStatus status = txValidator1.getStatus();
         assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR)));