Skip to content

Commit

Permalink
Merge pull request #2357 from ManfredKarrer/only-allow-grinbox-addresses
Browse files Browse the repository at this point in the history
Remove support for IP:port Grin address format.
  • Loading branch information
ManfredKarrer committed Feb 3, 2019
2 parents c4d561b + c11ec13 commit ac670e1
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 22 deletions.
18 changes: 7 additions & 11 deletions assets/src/main/java/bisq/asset/GrinAddressValidator.java
Expand Up @@ -18,7 +18,8 @@
package bisq.asset;

/**
* The supported "address" (better wallet URL) format is IP:port or the grinbox format.
* We only support the grinbox format as it is currently the only tool which offers a validation options of sender.
* Beside that is the IP:port format very insecure with MITM attacks.
*
* Here is the information from a conversation with the Grinbox developer regarding the Grinbox address format.
*
Expand Down Expand Up @@ -53,7 +54,6 @@ public class GrinAddressValidator implements AddressValidator {


// Regex for IP validation borrowed from https://stackoverflow.com/questions/53497/regular-expression-that-matches-valid-ipv6-addresses
private static final String IPV4 = "((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])";
private static final String PORT = "((6553[0-5])|(655[0-2][0-9])|(65[0-4][0-9]{2})|(6[0-4][0-9]{3})|([1-5][0-9]{4})|([0-5]{0,5})|([0-9]{1,4}))$";
private static final String DOMAIN = "[a-zA-Z0-9][a-zA-Z0-9-]{1,61}[a-zA-Z0-9]\\.[a-zA-Z]{2,}$";
private static final String KEY = "[a-km-zA-HJ-NP-Z1-9]{52}$";
Expand All @@ -64,13 +64,9 @@ public GrinAddressValidator() {
@Override
public AddressValidationResult validate(String address) {
if (address == null || address.length() == 0)
return AddressValidationResult.invalidAddress("Address may not be empty");
return AddressValidationResult.invalidAddress("Address may not be empty (only Grinbox format is supported)");

if (address.matches("^" + IPV4 + ":" + PORT))
return AddressValidationResult.validAddress();


// We might have a grinbox address
// We only support grinbox address
String key;
String domain = null;
String port = null;
Expand All @@ -92,13 +88,13 @@ public AddressValidationResult validate(String address) {
}

if (!key.matches("^" + KEY))
return AddressValidationResult.invalidAddress("Invalid key");
return AddressValidationResult.invalidAddress("Invalid key (only Grinbox format is supported)");

if (domain != null && !domain.matches("^" + DOMAIN))
return AddressValidationResult.invalidAddress("Invalid domain");
return AddressValidationResult.invalidAddress("Invalid domain (only Grinbox format is supported)");

if (port != null && !port.matches("^" + PORT))
return AddressValidationResult.invalidAddress("Invalid port");
return AddressValidationResult.invalidAddress("Invalid port (only Grinbox format is supported)");

return AddressValidationResult.validAddress();

Expand Down
2 changes: 2 additions & 0 deletions assets/src/main/java/bisq/asset/coins/Grin.java
Expand Up @@ -17,9 +17,11 @@

package bisq.asset.coins;

import bisq.asset.AltCoinAccountDisclaimer;
import bisq.asset.Coin;
import bisq.asset.GrinAddressValidator;

@AltCoinAccountDisclaimer("account.altcoin.popup.grin.msg")
public class Grin extends Coin {

public Grin() {
Expand Down
23 changes: 12 additions & 11 deletions assets/src/test/java/bisq/asset/coins/GrinTest.java
Expand Up @@ -29,16 +29,6 @@ public GrinTest() {

@Test
public void testValidAddresses() {
assertValidAddress("0.0.0.0:8080");
assertValidAddress("173.194.34.134:8080");
assertValidAddress("127.0.0.1:8080");
assertValidAddress("192.168.0.1:8080");
assertValidAddress("18.101.25.153:8080");
assertValidAddress("173.194.34.134:1");
assertValidAddress("173.194.34.134:11");
assertValidAddress("173.194.34.134:1111");
assertValidAddress("173.194.34.134:65535");

// grinbox
assertValidAddress("gVvk7rLBg3r3qoWYL3VsREnBbooT7nynxx5HtDvUWCJUaNCnddvY");
assertValidAddress("grinbox://gVtWzX5NTLCBkyNV19QVdnLXue13heAVRD36sfkGD6xpqy7k7e4a");
Expand All @@ -50,6 +40,18 @@ public void testValidAddresses() {

@Test
public void testInvalidAddresses() {
// valid IP:port addresses but not supported in Bisq
assertInvalidAddress("0.0.0.0:8080");
assertInvalidAddress("173.194.34.134:8080");
assertInvalidAddress("127.0.0.1:8080");
assertInvalidAddress("192.168.0.1:8080");
assertInvalidAddress("18.101.25.153:8080");
assertInvalidAddress("173.194.34.134:1");
assertInvalidAddress("173.194.34.134:11");
assertInvalidAddress("173.194.34.134:1111");
assertInvalidAddress("173.194.34.134:65535");

// invalid IP:port addresses
assertInvalidAddress("google.com");
assertInvalidAddress("100.100.100.100");
assertInvalidAddress(".100.100.100.100:1222");
Expand All @@ -71,6 +73,5 @@ public void testInvalidAddresses() {
assertInvalidAddress("gVw9TWimGFXRjoDXWhWxeNQbu84ZpLkvnenkKvA5aJeDo31eM5tC@somerelay.com:1220a");
assertInvalidAddress("grinbox://gVwjSsYW5vvHpK4AunJ5piKhhQTV6V3Jb818Uqs6PdC3SsB36AsAsomerelay.com");
assertInvalidAddress("grinbox://gVwjSsYW5vvHpK4AunJ5piKhhQTV6V3Jb818Uqs6PdC3SsB36AsA@somerelay.com1220");

}
}
6 changes: 6 additions & 0 deletions core/src/main/resources/i18n/displayStrings.properties
Expand Up @@ -1059,6 +1059,12 @@ account.altcoin.popup.ZEC.msg=When using Zcash you can only use the transparent
the z-addresses (private), because the arbitrator would not be able to verify the transaction with z-addresses.
account.altcoin.popup.XZC.msg=When using Zcoin you can only use the transparent (traceable) addresses not \
the untraceable addresses, because the arbitrator would not be able to verify the transaction with untraceable addresses at a block explorer.
account.altcoin.popup.grin.msg=Bisq supports only the Grinbox (Wallet713) wallet URL format. Please be sure that you use the \
latest Grinbox release and understand the process of transferring and receiving GRIN. \
The GRIN sender is required to provide a proof that they sent GRIN successfully. If they cannot provide the proof a \
potential dispute will be resolved in favor to the GRIN receiver. \n\n\
See https://github.com/vault713/wallet713/blob/master/docs/usage.md#transaction-proofs-grinbox-only for more \
information about the Grinbox proof tool.

account.fiat.yourFiatAccounts=Your national currency accounts

Expand Down

0 comments on commit ac670e1

Please sign in to comment.