From e53470f15ca6f6ca1a336fde220d1d2d249bd0b2 Mon Sep 17 00:00:00 2001 From: Kyle McCullen Date: Sat, 29 Aug 2026 10:36:27 -0400 Subject: [PATCH] prevent mismatch between qr code address and encrypted parameter --- .../V2/Services/BrantaServiceTests.cs | 101 ++++++++++++++++++ Branta/Exceptions/BrantaPaymentException.cs | 10 +- Branta/V2/Services/BrantaService.cs | 35 ++++-- CHANGELOG.md | 5 + 4 files changed, 141 insertions(+), 10 deletions(-) diff --git a/Branta.Tests/V2/Services/BrantaServiceTests.cs b/Branta.Tests/V2/Services/BrantaServiceTests.cs index 178857b..1735189 100644 --- a/Branta.Tests/V2/Services/BrantaServiceTests.cs +++ b/Branta.Tests/V2/Services/BrantaServiceTests.cs @@ -198,6 +198,107 @@ public async Task GetPaymentsByQrCodeAsync_CombinedZkQr_DecryptsBothAddressAndIn #endregion + #region GetPaymentsByQrCodeAsync address binding + + private const string SwappedAddress = "1BvBMSEYstWetqTFn5Au4m4GFg7xJaNVN2"; + private const string Bech32Address = "bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4"; + private const string EncryptedBech32Address = "encrypted-bech32-address"; + + private static Payment ZkBech32Payment => new PaymentBuilder() + .AddDestination(EncryptedBech32Address, type: DestinationType.BitcoinAddress) + .SetZk() + .Build(); + + [Fact] + public async Task GetPaymentsByQrCodeAsync_SwappedAddress_Rejects() + { + _clientMock + .Setup(c => c.GetPaymentsAsync(EncryptedBitcoinAddress, It.IsAny(), It.IsAny())) + .ReturnsAsync([ZkBitcoinPayment]); + + var qrText = $"bitcoin:{SwappedAddress}?branta_id={EncryptedBitcoinAddress}&branta_secret={Secret}"; + var ex = await Assert.ThrowsAsync(() => _service.GetPaymentsByQrCodeAsync(qrText)); + + Assert.Equal(BrantaPaymentExceptionReason.Tampered, ex.Reason); + } + + [Fact] + public async Task GetPaymentsByQrCodeAsync_MatchingAddress_DoesNotThrow() + { + _clientMock + .Setup(c => c.GetPaymentsAsync(EncryptedBitcoinAddress, It.IsAny(), It.IsAny())) + .ReturnsAsync([ZkBitcoinPayment]); + + var qrText = $"bitcoin:{BitcoinAddress}?branta_id={EncryptedBitcoinAddress}&branta_secret={Secret}"; + var result = await _service.GetPaymentsByQrCodeAsync(qrText); + + Assert.Equal(BitcoinAddress, result.Payments[0].Destinations[0].Value); + } + + [Fact] + public async Task GetPaymentsByQrCodeAsync_UppercaseBech32Qr_MatchesLowercaseRegistered_DoesNotThrow() + { + _aesEncryptionMock.Setup(e => e.Decrypt(EncryptedBech32Address, Secret)).Returns(Bech32Address); + _clientMock + .Setup(c => c.GetPaymentsAsync(EncryptedBech32Address, It.IsAny(), It.IsAny())) + .ReturnsAsync([ZkBech32Payment]); + + var qrText = $"bitcoin:{Bech32Address.ToUpperInvariant()}?branta_id={EncryptedBech32Address}&branta_secret={Secret}"; + var result = await _service.GetPaymentsByQrCodeAsync(qrText); + + Assert.Equal(Bech32Address, result.Payments[0].Destinations[0].Value); + } + + [Fact] + public async Task GetPaymentsByQrCodeAsync_Base58CaseMismatch_Rejects() + { + _clientMock + .Setup(c => c.GetPaymentsAsync(EncryptedBitcoinAddress, It.IsAny(), It.IsAny())) + .ReturnsAsync([ZkBitcoinPayment]); + + var qrText = $"bitcoin:{BitcoinAddress.ToLowerInvariant()}?branta_id={EncryptedBitcoinAddress}&branta_secret={Secret}"; + var ex = await Assert.ThrowsAsync(() => _service.GetPaymentsByQrCodeAsync(qrText)); + + Assert.Equal(BrantaPaymentExceptionReason.Tampered, ex.Reason); + } + + [Fact] + public async Task GetPaymentsByQrCodeAsync_LightningQrWithZkParams_NoPlainOnChainAddress_DecryptsWithoutComparison() + { + _clientMock + .Setup(c => c.GetPaymentsAsync(EncryptedBitcoinAddress, It.IsAny(), It.IsAny())) + .ReturnsAsync([ZkBitcoinPayment]); + + var qrText = $"lightning:{Bolt11Invoice}?branta_id={EncryptedBitcoinAddress}&branta_secret={Secret}"; + var result = await _service.GetPaymentsByQrCodeAsync(qrText); + + Assert.Equal(BitcoinAddress, result.Payments[0].Destinations[0].Value); + } + + [Fact] + public async Task GetPaymentsByQrCodeAsync_CombinedZkQr_SwappedAddress_Rejects() + { + var payment = new PaymentBuilder() + .AddDestination(EncryptedBitcoinAddress, type: DestinationType.BitcoinAddress) + .SetZk() + .AddDestination(EncryptedBolt11, type: DestinationType.Bolt11) + .SetZk() + .AddDestination(EncryptedArkAddress, type: DestinationType.ArkAddress) + .SetZk() + .Build(); + + _clientMock + .Setup(c => c.GetPaymentsAsync(EncryptedBitcoinAddress, It.IsAny(), It.IsAny())) + .ReturnsAsync([payment]); + + var qrText = $"bitcoin:{SwappedAddress}?branta_id={EncryptedBitcoinAddress}&branta_secret={Secret}&lightning={Bolt11Invoice}&ark={ArkAddress}"; + var ex = await Assert.ThrowsAsync(() => _service.GetPaymentsByQrCodeAsync(qrText)); + + Assert.Equal(BrantaPaymentExceptionReason.Tampered, ex.Reason); + } + + #endregion + #region GetPaymentsAsync [Fact] diff --git a/Branta/Exceptions/BrantaPaymentException.cs b/Branta/Exceptions/BrantaPaymentException.cs index 584d958..5bb1487 100644 --- a/Branta/Exceptions/BrantaPaymentException.cs +++ b/Branta/Exceptions/BrantaPaymentException.cs @@ -1,5 +1,11 @@ -namespace Branta.Exceptions; +namespace Branta.Exceptions; -public class BrantaPaymentException(string message) : Exception(message) +public enum BrantaPaymentExceptionReason { + Tampered +} + +public class BrantaPaymentException(string message, BrantaPaymentExceptionReason? reason = null) : Exception(message) +{ + public BrantaPaymentExceptionReason? Reason { get; } = reason; } diff --git a/Branta/V2/Services/BrantaService.cs b/Branta/V2/Services/BrantaService.cs index d02dcb6..179828a 100644 --- a/Branta/V2/Services/BrantaService.cs +++ b/Branta/V2/Services/BrantaService.cs @@ -14,6 +14,13 @@ public class BrantaService(IBrantaClient client, IAesEncryption aesEncryption, I private readonly BrantaClientOptions _defaultOptions = defaultOptions.Value; private readonly ISecretGenerator _secretGenerator = secretGenerator ?? new GuidSecretGenerator(); + private static bool AddressesMatch(string a, string b) + { + bool IsBech32(string v) => v.StartsWith("bc1", StringComparison.OrdinalIgnoreCase); + if (IsBech32(a) && IsBech32(b)) return string.Equals(a, b, StringComparison.OrdinalIgnoreCase); + return a == b; + } + public Task GetPaymentsByQrCodeAsync(string qrText, BrantaClientOptions? options = null, CancellationToken ct = default) { var parser = new QRParser(qrText); @@ -24,7 +31,8 @@ public Task GetPaymentsByQrCodeAsync(string qrText, BrantaClient .Where(d => d.Value.GetHashZkType().HasValue) .Select(d => d.Value) .ToList(); - return GetPaymentsForZkAsync(parser.OnChainEncryptionText!, parser.OnChainEncryptionSecret, additionalValues, options, ct); + var onChainAddress = parser.Destinations.FirstOrDefault(d => d.Type == DestinationType.BitcoinAddress)?.Value; + return GetPaymentsForZkAsync(parser.OnChainEncryptionText!, parser.OnChainEncryptionSecret, additionalValues, onChainAddress, options, ct); } var destination = parser.Destination!; @@ -34,14 +42,14 @@ public Task GetPaymentsByQrCodeAsync(string qrText, BrantaClient return GetPaymentsAsync(destination, null, options, ct); } - private async Task GetPaymentsForZkAsync(string lookupValue, string? encryptionKey, IReadOnlyList additionalHashValues, BrantaClientOptions? options, CancellationToken ct) + private async Task GetPaymentsForZkAsync(string lookupValue, string? encryptionKey, IReadOnlyList additionalHashValues, string? expectedOnChainAddress, BrantaClientOptions? options, CancellationToken ct) { var payments = await client.GetPaymentsAsync(lookupValue, options, ct); var keys = new Dictionary(); foreach (var payment in payments) { - DecryptDestinations(payment, lookupValue, encryptionKey, null, keys); + DecryptDestinations(payment, lookupValue, encryptionKey, null, keys, expectedOnChainAddress); foreach (var value in additionalHashValues) DecryptHashZkDestinations(payment, value, keys); } @@ -101,7 +109,7 @@ public async Task GetPaymentsAsync(string destinationValue, stri return new PaymentsResult { Payments = payments, VerifyUrl = BuildVerifyUrl(options, lookupValue, keys) }; } - private void DecryptDestinations(Payment payment, string destinationValue, string? encryptionKey, DestinationType? hashZkType, Dictionary keys) + private void DecryptDestinations(Payment payment, string destinationValue, string? encryptionKey, DestinationType? hashZkType, Dictionary keys, string? expectedOnChainAddress = null) { foreach (var destination in payment.Destinations) { @@ -111,17 +119,28 @@ private void DecryptDestinations(Payment payment, string destinationValue, strin if (destination.Type == DestinationType.BitcoinAddress) { if (encryptionKey == null) continue; + string decrypted; try { - destination.Value = aesEncryption.Decrypt(destination.Value, encryptionKey); - destination.IsEncrypted = false; - keys.TryAdd(destination.ZkId!, encryptionKey); - TryDecryptMetadata(payment, destination, encryptionKey); + decrypted = aesEncryption.Decrypt(destination.Value, encryptionKey); } catch { // Key didn't match this destination — leave it encrypted. + continue; } + + if (expectedOnChainAddress != null && !AddressesMatch(decrypted, expectedOnChainAddress)) + { + throw new BrantaPaymentException( + "The Bitcoin address in the QR code does not match the address verified by Branta. The QR code may have been tampered with.", + BrantaPaymentExceptionReason.Tampered); + } + + destination.Value = decrypted; + destination.IsEncrypted = false; + keys.TryAdd(destination.ZkId!, encryptionKey); + TryDecryptMetadata(payment, destination, encryptionKey); } else if (hashZkType.HasValue && destination.Type == hashZkType.Value) { diff --git a/CHANGELOG.md b/CHANGELOG.md index a278f4b..a8ac8dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed +- `GetPaymentsByQrCodeAsync` now verifies that the plaintext Bitcoin address parsed from a scanned QR code matches the address decrypted via `branta_id`/`branta_secret`, throwing `BrantaPaymentException` with `Reason: BrantaPaymentExceptionReason.Tampered` on mismatch. Closes a gap where an attacker could swap the visible address in a `bitcoin:` URI while leaving a legitimate, verified `branta_id`/`branta_secret` pair untouched (ported from `branta-js` 3.2.1) + ## [[3.2.0](https://github.com/BrantaOps/branta-dotnet/compare/3.1.6...3.2.0)] - 2026-07-17 ### Added