From fbe34015d437cf01c8ed7eeb4f042799a9d734ab Mon Sep 17 00:00:00 2001 From: CalDescent Date: Sat, 1 Jan 2022 22:33:01 +0000 Subject: [PATCH] Validate peer addresses before saving anything to the db. --- .../arbitrary/ArbitraryDataFileManager.java | 3 +++ .../arbitrary/ArbitraryDataManager.java | 6 ++++- .../data/network/ArbitraryPeerData.java | 23 +++++++++++++++++++ .../test/arbitrary/ArbitraryPeerTests.java | 4 ++++ 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/qortal/controller/arbitrary/ArbitraryDataFileManager.java b/src/main/java/org/qortal/controller/arbitrary/ArbitraryDataFileManager.java index d2ab6b87..344f9c75 100644 --- a/src/main/java/org/qortal/controller/arbitrary/ArbitraryDataFileManager.java +++ b/src/main/java/org/qortal/controller/arbitrary/ArbitraryDataFileManager.java @@ -137,6 +137,9 @@ public class ArbitraryDataFileManager { LOGGER.debug("Adding arbitrary peer: {} for signature {}", peerAddress, Base58.encode(signature)); ArbitraryPeerData arbitraryPeerData = new ArbitraryPeerData(signature, peer); repository.discardChanges(); + if (!arbitraryPeerData.isPeerAddressValid()) { + return false; + } repository.getArbitraryRepository().save(arbitraryPeerData); repository.saveChanges(); diff --git a/src/main/java/org/qortal/controller/arbitrary/ArbitraryDataManager.java b/src/main/java/org/qortal/controller/arbitrary/ArbitraryDataManager.java index 38e8783e..7d169bbf 100644 --- a/src/main/java/org/qortal/controller/arbitrary/ArbitraryDataManager.java +++ b/src/main/java/org/qortal/controller/arbitrary/ArbitraryDataManager.java @@ -396,7 +396,11 @@ public class ArbitraryDataManager extends Thread { // We haven't got a record of this mapping yet, so add it LOGGER.debug("Adding arbitrary peer: {} for signature {}", peerAddress, Base58.encode(signature)); ArbitraryPeerData arbitraryPeerData = new ArbitraryPeerData(signature, peer); - repository.getArbitraryRepository().save(arbitraryPeerData); + repository.discardChanges(); + if (!arbitraryPeerData.isPeerAddressValid()) { + return; + } + repository.getArbitraryRepository().save(arbitraryPeerData); repository.saveChanges(); // Remember that this data is new, so that it can be rebroadcast later diff --git a/src/main/java/org/qortal/data/network/ArbitraryPeerData.java b/src/main/java/org/qortal/data/network/ArbitraryPeerData.java index 3bd70939..2ec7b430 100644 --- a/src/main/java/org/qortal/data/network/ArbitraryPeerData.java +++ b/src/main/java/org/qortal/data/network/ArbitraryPeerData.java @@ -1,5 +1,6 @@ package org.qortal.data.network; +import com.google.common.net.InetAddresses; import org.qortal.crypto.Crypto; import org.qortal.network.Peer; import org.qortal.utils.NTP; @@ -28,6 +29,28 @@ public class ArbitraryPeerData { 0, 0, 0L, 0L); } + public boolean isPeerAddressValid() { + // Validate the peer address to prevent arbitrary values being added to the db + String[] parts = this.peerAddress.split(":"); + if (parts.length != 2) { + // Invalid format + return false; + } + String host = parts[0]; + if (!InetAddresses.isInetAddress(host)) { + // Invalid host + return false; + } + int port = Integer.valueOf(parts[1]); + if (port <= 0 || port > 65535) { + // Invalid port + return false; + } + + // Valid host/port combination + return true; + } + public void incrementSuccesses() { this.successes++; } diff --git a/src/test/java/org/qortal/test/arbitrary/ArbitraryPeerTests.java b/src/test/java/org/qortal/test/arbitrary/ArbitraryPeerTests.java index 05751ffd..ed7caa70 100644 --- a/src/test/java/org/qortal/test/arbitrary/ArbitraryPeerTests.java +++ b/src/test/java/org/qortal/test/arbitrary/ArbitraryPeerTests.java @@ -41,6 +41,7 @@ public class ArbitraryPeerTests extends Common { // Now add this mapping to the db Peer peer = new Peer(new PeerData(PeerAddress.fromString(peerAddress))); ArbitraryPeerData arbitraryPeerData = new ArbitraryPeerData(signature, peer); + assertTrue(arbitraryPeerData.isPeerAddressValid()); repository.getArbitraryRepository().save(arbitraryPeerData); // We should now have an entry for this hash/peer combination @@ -72,6 +73,7 @@ public class ArbitraryPeerTests extends Common { // Now add this mapping to the db Peer peer = new Peer(new PeerData(PeerAddress.fromString(peerAddress))); ArbitraryPeerData arbitraryPeerData = new ArbitraryPeerData(signature, peer); + assertTrue(arbitraryPeerData.isPeerAddressValid()); repository.getArbitraryRepository().save(arbitraryPeerData); // We should now have an entry for this hash/peer combination @@ -95,6 +97,7 @@ public class ArbitraryPeerTests extends Common { retrievedArbitraryPeerData.markAsAttempted(); Thread.sleep(100); retrievedArbitraryPeerData.markAsRetrieved(); + assertTrue(arbitraryPeerData.isPeerAddressValid()); repository.getArbitraryRepository().save(retrievedArbitraryPeerData); // Retrieve data once again @@ -135,6 +138,7 @@ public class ArbitraryPeerTests extends Common { // Now add this mapping to the db Peer peer = new Peer(new PeerData(PeerAddress.fromString(peerAddress1))); ArbitraryPeerData arbitraryPeerData = new ArbitraryPeerData(signature, peer); + assertTrue(arbitraryPeerData.isPeerAddressValid()); repository.getArbitraryRepository().save(arbitraryPeerData); // We should now have an entry for this hash/peer combination