SFT-3834: ensured preview address matches the signed output address
What changed, and why it matters
This firmware update fixes a bug where the Bitcoin address shown on the Passport hardware wallet screen before signing could differ from the address actually used to create the signature. The fix makes the signing task verify that the address it computes matches the address previewed to the user, and aborts if they don't match. This prevents a user from being tricked into approving a signature for one address while the device signs with another.
Treat this as a security-hardening fix and include it in the next firmware release. Review other signing flows for similar preview-vs-actual address divergence issues. No immediate user action is required beyond updating firmware when available.
Security signals we found
Address mismatch check added between previewed and signed addresses
Separation of display-formatted address from canonical address used for signing
Refactoring to consolidate signing logic through HealthCheckCommonFlow
Potential user-confusion/tricked-approval issue mitigated
Evidence from the diff
The commit refactors health-check signing flows and adds an expected_address parameter to sign_text_file_task. The preview flow now keeps a separate display_address (stylized) while passing the raw computed self.address into the signing task. The task derives the address again from the subpath and address format, compares it to expected_address, and returns an ‘Address mismatch’ error instead of signing if they differ. This closes a window where stylization or flow state could cause the displayed address to diverge from the signed output address.
Changed components
ports/stm32/boards/Passport/modules/flows/health_check_common_flow.pyports/stm32/boards/Passport/modules/flows/health_check_qr_flow.pyports/stm32/boards/Passport/modules/tasks/sign_text_file_task.pyInspect captured patch +19 / −46
diff --git a/ports/stm32/boards/Passport/modules/flows/health_check_common_flow.py b/ports/stm32/boards/Passport/modules/flows/health_check_common_flow.py
index 45a0562..4981dc9 100644
--- a/ports/stm32/boards/Passport/modules/flows/health_check_common_flow.py
+++ b/ports/stm32/boards/Passport/modules/flows/health_check_common_flow.py
@@ -61,7 +61,7 @@ class HealthCheckCommonFlow(Flow):
node = sv.derive_path(self.subpath)
self.address = sv.chain.address(node, self.addr_type)
- self.address = stylize_address(self.address)
+ display_address = stylize_address(self.address)
result = await LongTextPage(centered=True,
text=('\n' + self.text),
@@ -71,7 +71,7 @@ class HealthCheckCommonFlow(Flow):
self.set_result(False)
return
- result = await LongQuestionPage(text='Sign message with this address?\n\n{}'.format(self.address),
+ result = await LongQuestionPage(text='Sign message with this address?\n\n{}'.format(display_address),
right_micron=microns.Sign,
margins=MARGIN_FOR_ADDRESSES,
top_margin=8).show()
@@ -88,7 +88,7 @@ class HealthCheckCommonFlow(Flow):
from utils import spinner_task
text = 'Signing message' if self.normal_signing else 'Performing health check'
(signature, address, error) = await spinner_task(text, sign_text_file_task,
- args=[self.text, self.subpath, self.addr_type])
+ args=[self.text, self.subpath, self.addr_type, self.address if self.normal_signing else None])
if error is None:
self.signature = signature
self.address = address
diff --git a/ports/stm32/boards/Passport/modules/flows/health_check_qr_flow.py b/ports/stm32/boards/Passport/modules/flows/health_check_qr_flow.py
index 758e223..3f6ea41 100644
--- a/ports/stm32/boards/Passport/modules/flows/health_check_qr_flow.py
+++ b/ports/stm32/boards/Passport/modules/flows/health_check_qr_flow.py
@@ -4,11 +4,6 @@
# health_check_flow.py - Scan and process a health check QR code in `crypto-request` format
from flows import Flow
-from pages import ErrorPage, SuccessPage
-from pages.show_qr_page import ShowQRPage
-from utils import validate_sign_text, spinner_task
-from tasks import sign_text_file_task
-from public_constants import AF_CLASSIC, RFC_SIGNATURE_TEMPLATE
from data_codecs.qr_type import QRType
from foundation import ur
@@ -18,8 +13,8 @@ class HealthCheckQRFlow(Flow):
super().__init__(initial_state=self.scan_qr, name='HealthCheckQRFlow')
self.service_name = context
- self.text = None
- self.subpath = None
+ self.lines = None
+ self.signed_message = None
async def scan_qr(self):
from pages import ErrorPage
@@ -35,55 +30,29 @@ class HealthCheckQRFlow(Flow):
try:
data = result.unwrap_bytes().decode('utf-8')
- lines = data.split('\n')
- if len(lines) != 2:
- await ErrorPage('Health check format is invalid.').show()
- self.set_result(False)
- return
-
- self.text = lines[0]
- self.subpath = lines[1]
+ self.lines = data.split('\n')
except Exception as e:
await ErrorPage('Health check format is invalid.').show()
self.set_result(False)
return
- # Validate
- (subpath, error) = validate_sign_text(self.text, self.subpath)
- if error is not None:
- await ErrorPage(text=error).show()
- self.set_result(False)
- return
+ self.goto(self.common_flow)
- self.subpath = subpath
- self.goto(self.sign_health_check)
+ async def common_flow(self):
+ from flows import HealthCheckCommonFlow
- async def sign_health_check(self):
- (signature, address, error) = await spinner_task('Performing Health Check',
- sign_text_file_task,
- args=[self.text, self.subpath, AF_CLASSIC])
- if error is None:
- self.signature = signature
- self.address = address
- self.goto(self.show_signed_message, save_curr=False)
- else:
- await ErrorPage(text='Error while signing file: {}'.format(error)).show()
+ self.signed_message = await HealthCheckCommonFlow(self.lines).run()
+ if self.signed_message is None:
self.set_result(False)
return
+ self.goto(self.show_signed_message)
async def show_signed_message(self):
- from ubinascii import b2a_base64
-
- sig = b2a_base64(self.signature).decode('ascii').strip()
-
- signed_message = ur.new_bytes(RFC_SIGNATURE_TEMPLATE.format(addr=self.address,
- msg=self.text,
- blockchain='BITCOIN',
- sig=sig))
+ from pages import ShowQRPage
result = await ShowQRPage(
qr_type=QRType.UR2,
- qr_data=signed_message,
+ qr_data=ur.new_bytes(self.signed_message),
caption='Signed Health Check'
).show()
if not result:
diff --git a/ports/stm32/boards/Passport/modules/tasks/sign_text_file_task.py b/ports/stm32/boards/Passport/modules/tasks/sign_text_file_task.py
index d167a54..89dd303 100644
--- a/ports/stm32/boards/Passport/modules/tasks/sign_text_file_task.py
+++ b/ports/stm32/boards/Passport/modules/tasks/sign_text_file_task.py
@@ -14,12 +14,16 @@ import chains
from utils import sign_message_digest_recoverable
-async def sign_text_file_task(on_done, text, subpath, addr_fmt):
+async def sign_text_file_task(on_done, text, subpath, addr_fmt, expected_address=None):
with stash.SensitiveValues() as sv:
node = sv.derive_path(subpath)
address = sv.chain.address(node, addr_fmt)
+ if expected_address is not None and address != expected_address:
+ await on_done(None, None, 'Address mismatch: expected {}, got {}'.format(expected_address, address))
+ return
+
digest = chains.current_chain().hash_message(text.encode())
# signature will be 65 bytes
signature = sign_message_digest_recoverable(digest, subpath)
Why this scored 59/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.