Reject withdrawal addresses that aren't PLM
embit's Script.from_address accepts a well-formed bech32 address from any chain: a Bitcoin bc1... parses into a perfectly valid witness program. So a withdrawal to a BTC address built, signed and broadcast normally on PLM, and the funds landed on a script nobody holds the key for — silently, with no error anywhere. A malformed address fared slightly better only in that it crashed the request with an unhandled 500. is_valid_plm_address checks the HRP as well as the parse, and runs first in request_withdrawal, before a single UTXO is touched. It matches what the withdrawal form already told the user (bech32 plm1q... only). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -122,6 +122,7 @@ const TRANSLATIONS = {
|
||||
'error.already_betting': 'You already have an active bet in the current round.',
|
||||
'error.insufficient_balance': 'Insufficient balance.',
|
||||
'error.amount_below_network_fee': 'The amount is too small to cover the network fee.',
|
||||
'error.invalid_address': 'Not a valid PLM address (it must start with plm1q…).',
|
||||
'error.amount_below_minimum': 'The minimum withdrawal amount is {minimum_plm} PLM.',
|
||||
'error.current_password_incorrect': 'The current password is incorrect.',
|
||||
'error.password_too_short': 'The new password must be at least {minimum} characters.',
|
||||
@@ -251,6 +252,7 @@ const TRANSLATIONS = {
|
||||
'error.already_betting': 'Hai già una bet attiva nel round corrente.',
|
||||
'error.insufficient_balance': 'Saldo insufficiente.',
|
||||
'error.amount_below_network_fee': "L'importo è troppo basso per coprire la fee di rete.",
|
||||
'error.invalid_address': 'Indirizzo PLM non valido (deve iniziare con plm1q…).',
|
||||
'error.amount_below_minimum': "L'importo minimo di prelievo è {minimum_plm} PLM.",
|
||||
'error.current_password_incorrect': 'La password attuale non è corretta.',
|
||||
'error.password_too_short': 'La nuova password deve avere almeno {minimum} caratteri.',
|
||||
@@ -380,6 +382,7 @@ const TRANSLATIONS = {
|
||||
'error.already_betting': 'Ya tienes una apuesta activa en la ronda actual.',
|
||||
'error.insufficient_balance': 'Saldo insuficiente.',
|
||||
'error.amount_below_network_fee': 'El importe es demasiado pequeño para cubrir la comisión de red.',
|
||||
'error.invalid_address': 'Dirección PLM no válida (debe empezar por plm1q…).',
|
||||
'error.amount_below_minimum': 'El importe mínimo de retiro es {minimum_plm} PLM.',
|
||||
'error.current_password_incorrect': 'La contraseña actual no es correcta.',
|
||||
'error.password_too_short': 'La nueva contraseña debe tener al menos {minimum} caracteres.',
|
||||
@@ -509,6 +512,7 @@ const TRANSLATIONS = {
|
||||
'error.already_betting': 'Vous avez déjà une mise active dans le round en cours.',
|
||||
'error.insufficient_balance': 'Solde insuffisant.',
|
||||
'error.amount_below_network_fee': 'Le montant est trop faible pour couvrir les frais de réseau.',
|
||||
'error.invalid_address': 'Adresse PLM invalide (elle doit commencer par plm1q…).',
|
||||
'error.amount_below_minimum': 'Le montant minimum de retrait est de {minimum_plm} PLM.',
|
||||
'error.current_password_incorrect': "Le mot de passe actuel n'est pas correct.",
|
||||
'error.password_too_short': 'Le nouveau mot de passe doit comporter au moins {minimum} caractères.',
|
||||
@@ -638,6 +642,7 @@ const TRANSLATIONS = {
|
||||
'error.already_betting': 'Du hast bereits eine aktive Wette in der laufenden Runde.',
|
||||
'error.insufficient_balance': 'Nicht genügend Guthaben.',
|
||||
'error.amount_below_network_fee': 'Der Betrag ist zu klein, um die Netzwerkgebühr zu decken.',
|
||||
'error.invalid_address': 'Keine gültige PLM-Adresse (sie muss mit plm1q… beginnen).',
|
||||
'error.amount_below_minimum': 'Der Mindestauszahlungsbetrag beträgt {minimum_plm} PLM.',
|
||||
'error.current_password_incorrect': 'Das aktuelle Passwort ist nicht korrekt.',
|
||||
'error.password_too_short': 'Das neue Passwort muss mindestens {minimum} Zeichen lang sein.',
|
||||
@@ -767,6 +772,7 @@ const TRANSLATIONS = {
|
||||
'error.already_betting': 'У вас уже есть активная ставка в текущем раунде.',
|
||||
'error.insufficient_balance': 'Недостаточно средств.',
|
||||
'error.amount_below_network_fee': 'Сумма слишком мала, чтобы покрыть комиссию сети.',
|
||||
'error.invalid_address': 'Некорректный адрес PLM (он должен начинаться с plm1q…).',
|
||||
'error.amount_below_minimum': 'Минимальная сумма вывода — {minimum_plm} PLM.',
|
||||
'error.current_password_incorrect': 'Текущий пароль указан неверно.',
|
||||
'error.password_too_short': 'Новый пароль должен содержать не менее {minimum} символов.',
|
||||
@@ -896,6 +902,7 @@ const TRANSLATIONS = {
|
||||
'error.already_betting': '你在当前回合已有一笔有效下注。',
|
||||
'error.insufficient_balance': '余额不足。',
|
||||
'error.amount_below_network_fee': '金额太小,不足以支付网络手续费。',
|
||||
'error.invalid_address': 'PLM 地址无效(必须以 plm1q… 开头)。',
|
||||
'error.amount_below_minimum': '最低提现金额为 {minimum_plm} PLM。',
|
||||
'error.current_password_incorrect': '当前密码不正确。',
|
||||
'error.password_too_short': '新密码长度至少需要 {minimum} 个字符。',
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
"""Validation for PLM addresses supplied by the user (withdrawal destinations).
|
||||
|
||||
embit's `Script.from_address` accepts a well-formed bech32 address from *any*
|
||||
chain — a Bitcoin `bc1...` parses fine and yields a perfectly valid witness
|
||||
program — so parse-success alone is not a sufficient check here: a withdrawal
|
||||
to a `bc1...` address would build, sign and broadcast normally on PLM and land
|
||||
on a script nobody holds the key for. The HRP check below is what makes the
|
||||
destination actually PLM, and it matches what the withdrawal form already
|
||||
tells the user (bech32 `plm1q...` only).
|
||||
"""
|
||||
|
||||
from embit import script
|
||||
from embit.base import EmbitError
|
||||
|
||||
from app.wallet.plm_network import PLM_MAINNET
|
||||
|
||||
_BECH32_PREFIX = PLM_MAINNET["bech32"] + "1"
|
||||
|
||||
|
||||
def is_valid_plm_address(address: str) -> bool:
|
||||
if not address.startswith(_BECH32_PREFIX):
|
||||
return False
|
||||
try:
|
||||
script.Script.from_address(address)
|
||||
except EmbitError:
|
||||
return False
|
||||
return True
|
||||
@@ -8,6 +8,7 @@ from app.db.models import PendingTransaction, User, UtxoEvent, Withdrawal
|
||||
from app.electrum.client import ElectrumClient
|
||||
from app.rounds.config import get_round_config
|
||||
from app.rounds.events import broadcaster
|
||||
from app.wallet.address import is_valid_plm_address
|
||||
from app.wallet.balance import recompute_balance
|
||||
from app.wallet.hd import derive_user_key
|
||||
from app.wallet.psbt_builder import InsufficientFundsError, Utxo, build_signed_transaction
|
||||
@@ -20,6 +21,12 @@ class WithdrawalError(ApiError):
|
||||
async def request_withdrawal(
|
||||
session: AsyncSession, client: ElectrumClient, user: User, external_address: str, amount_sats: int
|
||||
) -> Withdrawal:
|
||||
# Checked before anything else: an address from another chain parses fine as a
|
||||
# witness program (see wallet/address.py), so without this the tx would build,
|
||||
# broadcast and be irrecoverable rather than fail.
|
||||
if not is_valid_plm_address(external_address):
|
||||
raise WithdrawalError("invalid_address", "not a valid PLM bech32 address")
|
||||
|
||||
config = await get_round_config(session)
|
||||
if amount_sats < config.bet_amount_sats:
|
||||
raise WithdrawalError(
|
||||
|
||||
@@ -97,3 +97,28 @@ async def test_withdrawal_rejects_insufficient_balance(session_factory):
|
||||
user = await session.get(User, user_id)
|
||||
with pytest.raises(WithdrawalError, match="insufficient balance"):
|
||||
await request_withdrawal(session, client, user, EXTERNAL_ADDRESS, BET_AMOUNT_SATS)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"address",
|
||||
[
|
||||
"bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4", # valid bech32, wrong chain
|
||||
"plm1qbogus", # right HRP, broken checksum
|
||||
"not-an-address",
|
||||
],
|
||||
)
|
||||
async def test_withdrawal_rejects_non_plm_address(session_factory, address):
|
||||
"""The bc1 case is the one that matters: embit parses it into a perfectly
|
||||
valid witness program, so without the HRP check the withdrawal would build,
|
||||
sign and broadcast on PLM, sending the funds somewhere nobody holds a key
|
||||
for. It has to fail before a single UTXO is touched."""
|
||||
user_id = await _make_funded_user(session_factory, 3, 2_000_000_000)
|
||||
client = FakeElectrumClient()
|
||||
|
||||
async with session_factory() as session:
|
||||
user = await session.get(User, user_id)
|
||||
with pytest.raises(WithdrawalError) as exc_info:
|
||||
await request_withdrawal(session, client, user, address, BET_AMOUNT_SATS)
|
||||
|
||||
assert exc_info.value.code == "invalid_address"
|
||||
assert not client.broadcasted
|
||||
|
||||
Reference in New Issue
Block a user