bugfix: multisig ascii input validation
(cherry picked from commit 2d2f8f3d9d)
This commit is contained in:
scgbckbone
doc-hex
parent
9890b0a8b9
commit
4e6881c088
@ -3,6 +3,8 @@
|
||||
- Bugfix: Saving passphrase on SD Card caused a freeze that required reboot
|
||||
- Bugfix: Properly handle and finalize framing error response
|
||||
- Bugfix: `Brick Me` option for `If Wrong` PIN caused yikes
|
||||
- Bugfix: Do not allow non-ascii or ascii non-printable characters in multisig
|
||||
wallet name
|
||||
|
||||
## 5.2.2 - 2023-12-21
|
||||
|
||||
|
||||
@ -15,7 +15,7 @@ from ux import ux_aborted, ux_show_story, abort_and_goto, ux_dramatic_pause, ux_
|
||||
from ux import show_qr_code
|
||||
from usb import CCBusyError
|
||||
from utils import HexWriter, xfp2str, problem_file_line, cleanup_deriv_path
|
||||
from utils import B2A, parse_addr_fmt_str
|
||||
from utils import B2A, parse_addr_fmt_str, to_ascii_printable
|
||||
from psbt import psbtObject, FatalPSBTIssue, FraudulentChangeOutput
|
||||
from exceptions import HSMDenied
|
||||
from version import MAX_TXN_LEN
|
||||
@ -279,27 +279,14 @@ def validate_text_for_signing(text):
|
||||
# - messages must be short and ascii only. Our charset is limited
|
||||
# - too many spaces, leading/trailing can be an issue
|
||||
|
||||
MSG_CHARSET = range(32, 127)
|
||||
MSG_MAX_SPACES = 4 # impt. compared to -=- positioning
|
||||
|
||||
try:
|
||||
result = str(text, 'ascii')
|
||||
except UnicodeError:
|
||||
raise AssertionError('must be ascii')
|
||||
result = to_ascii_printable(text)
|
||||
|
||||
length = len(result)
|
||||
assert length >= 2, "msg too short (min. 2)"
|
||||
assert length <= MSG_SIGNING_MAX_LENGTH, "msg too long (max. %d)" % MSG_SIGNING_MAX_LENGTH
|
||||
run = 0
|
||||
for ch in result:
|
||||
assert ord(ch) in MSG_CHARSET, "bad char: 0x%02x in msg" % ord(ch)
|
||||
|
||||
if ch == ' ':
|
||||
run += 1
|
||||
assert run < MSG_MAX_SPACES, 'too many spaces together in msg(max. 4)'
|
||||
else:
|
||||
run = 0
|
||||
|
||||
assert " " not in result, 'too many spaces together in msg(max. 3)'
|
||||
# other confusion w/ whitepace
|
||||
assert result[0] != ' ', 'leading space(s) in msg'
|
||||
assert result[-1] != ' ', 'trailing space(s) in msg'
|
||||
|
||||
@ -3,7 +3,7 @@
|
||||
# multisig.py - support code for multisig signing and p2sh in general.
|
||||
#
|
||||
import stash, chains, ustruct, ure, uio, sys, ngu, uos, ujson
|
||||
from utils import xfp2str, str2xfp, swab32, cleanup_deriv_path, keypath_to_str
|
||||
from utils import xfp2str, str2xfp, swab32, cleanup_deriv_path, keypath_to_str, to_ascii_printable
|
||||
from utils import str_to_keypath, problem_file_line, parse_extended_key
|
||||
from ux import ux_show_story, ux_confirm, ux_dramatic_pause, ux_clear_keys, ux_enter_bip32_index
|
||||
from files import CardSlot, CardMissingError, needs_microsd
|
||||
@ -716,7 +716,7 @@ class MultisigWallet:
|
||||
name = '%d-of-%d' % (M, N)
|
||||
|
||||
try:
|
||||
name = str(name, 'ascii')
|
||||
name = to_ascii_printable(name)
|
||||
assert 1 <= len(name) <= 20
|
||||
except:
|
||||
raise AssertionError('name must be ascii, 1..20 long')
|
||||
|
||||
@ -186,6 +186,30 @@ def str2xfp(txt):
|
||||
# Inverse of xfp2str
|
||||
return ustruct.unpack('<I', a2b_hex(txt))[0]
|
||||
|
||||
def is_ascii(s):
|
||||
if len(s) == len(s.encode()):
|
||||
return True
|
||||
return False
|
||||
|
||||
def is_printable(s):
|
||||
PRINTABLE = range(32, 127)
|
||||
for ch in s:
|
||||
if ord(ch) not in PRINTABLE:
|
||||
return False
|
||||
return True
|
||||
|
||||
def to_ascii_printable(s, strip=False):
|
||||
try:
|
||||
s = str(s, 'ascii')
|
||||
if strip:
|
||||
s = s.strip()
|
||||
assert is_ascii(s)
|
||||
assert is_printable(s)
|
||||
return s
|
||||
except:
|
||||
raise AssertionError('must be ascii printable')
|
||||
|
||||
|
||||
def problem_file_line(exc):
|
||||
# return a string of just the filename.py and line number where
|
||||
# an exception occured. Best used on AssertionError.
|
||||
@ -222,10 +246,8 @@ def cleanup_deriv_path(bin_path, allow_star=False):
|
||||
# - if allow_star, then final position can be * or *' (wildcard)
|
||||
import ure
|
||||
from public_constants import MAX_PATH_DEPTH
|
||||
try:
|
||||
s = str(bin_path, 'ascii').lower()
|
||||
except UnicodeError:
|
||||
raise AssertionError('must be ascii')
|
||||
|
||||
s = to_ascii_printable(bin_path, strip=True).lower()
|
||||
|
||||
# empty string is valid
|
||||
if s == '': return 'm'
|
||||
|
||||
@ -65,7 +65,7 @@ def test_sign_msg_refused(dev, press_cancel):
|
||||
('23234pp', 'bad component'),
|
||||
("23234p'", 'bad component'),
|
||||
("m/1p/3455343434443534543345p", 'bad component'),
|
||||
("m/\n34p", 'invalid characters'),
|
||||
("m/\n34p", 'must be ascii printable'),
|
||||
("2147483648/1/2/3", 'bad component'), # 2**31 = 0x80000000 not allowed (because that's 0')
|
||||
("214748364800/1/2/3", 'bad component'),
|
||||
('/'.join('0' for i in range(13)), 'too deep'),
|
||||
@ -212,14 +212,19 @@ def sign_using_nfc(goto_home, pick_menu_item, nfc_write_text, cap_story):
|
||||
('hello%20sworld'%'', 'many spaces', 0), # spaces
|
||||
('hello%10sworld'%'', 'many spaces', 0), # spaces
|
||||
('hello%5sworld'%'', 'many spaces', 0), # spaces
|
||||
('test\ttest', "bad char: 0x09", 0),
|
||||
])
|
||||
('test\ttest', "must be ascii printable", 0),
|
||||
('testêtest', "must be ascii printable", 0),
|
||||
])
|
||||
@pytest.mark.parametrize('transport', ['sd', 'usb', 'nfc'])
|
||||
def test_sign_msg_fails(dev, sign_on_microsd, msg, concern, no_file, transport, sign_using_nfc, path='m/12/34'):
|
||||
|
||||
if transport == 'usb':
|
||||
with pytest.raises(CCProtoError) as ee:
|
||||
dev.send_recv(CCProtocolPacker.sign_message(msg.encode('ascii'), path), timeout=None)
|
||||
try:
|
||||
encoded_msg = msg.encode('ascii')
|
||||
except UnicodeEncodeError:
|
||||
encoded_msg = msg.encode()
|
||||
dev.send_recv(CCProtocolPacker.sign_message(encoded_msg, path), timeout=None)
|
||||
story = ee.value.args[0]
|
||||
elif transport == 'sd':
|
||||
try:
|
||||
|
||||
@ -144,9 +144,9 @@ def make_multisig(dev, sim_execfile):
|
||||
|
||||
@pytest.fixture
|
||||
def offer_ms_import(cap_story, dev):
|
||||
def doit(config):
|
||||
def doit(config, allow_non_ascii=False):
|
||||
# upload the file, trigger import
|
||||
file_len, sha = dev.upload_file(config.encode('ascii'))
|
||||
file_len, sha = dev.upload_file(config.encode('utf-8' if allow_non_ascii else 'ascii'))
|
||||
|
||||
open('debug/last-config.txt', 'wt').write(config)
|
||||
|
||||
@ -241,7 +241,7 @@ def import_ms_wallet(dev, make_multisig, offer_ms_import, press_select, is_q1):
|
||||
def test_ms_import_variations(N, make_multisig, offer_ms_import, press_cancel, is_q1):
|
||||
# all the different ways...
|
||||
keys = make_multisig(N, N)
|
||||
|
||||
|
||||
|
||||
# bare, no fingerprints
|
||||
# - no xfps
|
||||
@ -436,6 +436,7 @@ def test_ms_show_addr(dev, cap_story, press_select, addr_vs_path, bitcoind_p2sh,
|
||||
assert B2A(scr) == core_scr
|
||||
assert core_addr == got_addr
|
||||
|
||||
|
||||
return doit
|
||||
|
||||
|
||||
@ -590,7 +591,7 @@ def test_bad_common_prefix(cpp, use_regtest, clear_ms, import_ms_wallet,
|
||||
def test_import_detail(clear_ms, import_ms_wallet, need_keypress,
|
||||
cap_story, is_q1, press_cancel):
|
||||
# check all details are shown right
|
||||
|
||||
|
||||
M,N = 14, 15
|
||||
|
||||
keys = import_ms_wallet(M, N)
|
||||
@ -619,7 +620,7 @@ def test_import_detail(clear_ms, import_ms_wallet, need_keypress,
|
||||
def test_export_airgap(acct_num, goto_home, cap_story, pick_menu_item, cap_menu,
|
||||
need_keypress, microsd_path, load_export, use_mainnet,
|
||||
testnet, way, is_q1, press_select):
|
||||
|
||||
|
||||
|
||||
if not testnet:
|
||||
use_mainnet()
|
||||
@ -828,7 +829,7 @@ def test_export_single_ux(goto_home, comm_prefix, cap_story, pick_menu_item, cap
|
||||
|
||||
@pytest.mark.parametrize('N', [ 3, 15])
|
||||
def test_overflow(N, import_ms_wallet, clear_ms, press_select, cap_story, mk_num, is_q1):
|
||||
|
||||
|
||||
clear_ms()
|
||||
M = N
|
||||
name = 'a'*20 # longest possible
|
||||
@ -886,7 +887,7 @@ def test_import_dup_safe(N, clear_ms, make_multisig, offer_ms_import,
|
||||
need_keypress, cap_story, goto_home, pick_menu_item,
|
||||
cap_menu, is_q1, press_select):
|
||||
# import wallet, rename it, (check that indicated, works), attempt same w/ addr fmt different
|
||||
|
||||
|
||||
M = N
|
||||
|
||||
clear_ms()
|
||||
@ -1072,7 +1073,7 @@ def make_myself_wallet(dev, set_bip39_pw, offer_ms_import, press_select, clear_m
|
||||
|
||||
# construct a wallet (M of 4) using different bip39 passwords, and default sim
|
||||
def doit(M, addr_fmt=None, do_import=True):
|
||||
|
||||
|
||||
passwords = ['Me', 'Myself', 'And I', '']
|
||||
|
||||
if 0:
|
||||
@ -1357,8 +1358,6 @@ def test_make_airgapped(addr_fmt, acct_num, N, goto_home, cap_story, pick_menu_i
|
||||
need_keypress, microsd_path, set_bip39_pw, clear_ms,
|
||||
get_settings, load_export, is_q1, press_select, press_cancel):
|
||||
# test UX and math for bip45 export
|
||||
|
||||
|
||||
|
||||
# cleanup
|
||||
from glob import glob
|
||||
@ -1814,7 +1813,7 @@ def test_ms_import_many_derivs(M, N, way, make_multisig, clear_ms, offer_ms_impo
|
||||
goto_home, load_export, is_q1):
|
||||
# try config file with different derivation paths given, including None
|
||||
# - also check we can convert those into Electrum wallets
|
||||
|
||||
|
||||
actual = "m/48'/0'/0'/1'/0"
|
||||
derivs = [ actual, 'm', "m/45'/0'/99'", "m/45'/34/34'/34"]
|
||||
|
||||
@ -1983,7 +1982,7 @@ def test_dup_ms_wallet_bug(goto_home, pick_menu_item, press_select, import_ms_wa
|
||||
clear_ms, is_q1):
|
||||
M = 2
|
||||
N = 3
|
||||
|
||||
|
||||
deriv = ["m/48'/1'/0'/69'/1"]*N
|
||||
fmts = [ 'p2wsh', 'p2sh-p2wsh']
|
||||
|
||||
@ -2142,7 +2141,7 @@ def test_bitcoind_ms_address(change, descriptor, M_N, addr_fmt, clear_ms, goto_h
|
||||
def test_legacy_multisig_witness_utxo_in_psbt(bitcoind, use_regtest, clear_ms, microsd_wipe, goto_home, need_keypress,
|
||||
pick_menu_item, cap_story, load_export, microsd_path, cap_menu, try_sign,
|
||||
is_q1, press_select):
|
||||
|
||||
|
||||
use_regtest()
|
||||
clear_ms()
|
||||
microsd_wipe()
|
||||
@ -2255,7 +2254,7 @@ def test_bitcoind_MofN_tutorial(m_n, desc_type, clear_ms, goto_home, need_keypre
|
||||
microsd_wipe, load_export, settings_set, psbt_v2, is_q1,
|
||||
finalize_v2_v0_convert, press_select):
|
||||
# 2of2 case here is described in docs with tutorial
|
||||
|
||||
|
||||
M, N = m_n
|
||||
settings_set("sighshchk", 1) # disable checks
|
||||
use_regtest()
|
||||
@ -2680,4 +2679,21 @@ def test_multisig_descriptor_export(M_N, way, addr_fmt, cmn_pth_from_root, clear
|
||||
assert obj["desc"] == bare_desc
|
||||
clear_ms()
|
||||
|
||||
|
||||
def test_multisig_name_validation(microsd_path, offer_ms_import):
|
||||
with open("data/multisig/export-p2wsh-myself.txt", "r") as f:
|
||||
config = f.read()
|
||||
|
||||
c0 = config.replace("Name: CC-2-of-4", "Name: eê")
|
||||
|
||||
with pytest.raises(Exception) as e:
|
||||
offer_ms_import(c0, allow_non_ascii=True)
|
||||
assert "must be ascii" in e.value.args[0]
|
||||
|
||||
c0 = config.replace("Name: CC-2-of-4", "Name: eee\teee")
|
||||
|
||||
with pytest.raises(Exception) as e:
|
||||
offer_ms_import(c0, allow_non_ascii=True)
|
||||
assert "must be ascii" in e.value.args[0]
|
||||
|
||||
# EOF
|
||||
|
||||
Loading…
Reference in New Issue
Block a user