Skip to content

Fix: Handle UTF-8 decode error and overflow error in wallet storage #9564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix: Handle UTF-8 decode error and overflow error in wallet storage #9564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 71 additions & 64 deletions electrum/gui/qt/wizard/wallet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,72 +310,79 @@ def on_choose():
if _path:
self.name_e.setText(relative_path(_path))

def on_filename(filename):
# FIXME? "filename" might contain ".." (etc) and hence sketchy path traversals are possible
nonlocal temp_storage
temp_storage = None
msg = None
self.wallet_exists = False
self.wallet_is_open = False
self.wallet_needs_hw_unlock = False
if filename:
_path = os.path.join(datadir_wallet_folder, filename)
wallet_from_memory = self.wizard._daemon.get_wallet(_path)
try:
if wallet_from_memory:
temp_storage = wallet_from_memory.storage # type: Optional[WalletStorage]
self.wallet_is_open = True
else:
temp_storage = WalletStorage(_path)
self.wallet_exists = temp_storage.file_exists()
except (StorageReadWriteError, WalletFileException) as e:
msg = _('Cannot read file') + f'\n{repr(e)}'
except Exception as e:
self.logger.exception('')
msg = _('Cannot read file') + f'\n{repr(e)}'
else:
msg = ""
self.valid = temp_storage is not None
user_needs_to_enter_password = False
if temp_storage:
if not temp_storage.file_exists():
msg = _("This file does not exist.") + '\n' \
+ _("Press 'Next' to create this wallet, or choose another file.")
elif not wallet_from_memory:
if temp_storage.is_encrypted_with_user_pw():
msg = _("This file is encrypted with a password.") + '\n' \
+ _('Enter your password or choose another file.')
user_needs_to_enter_password = True
elif temp_storage.is_encrypted_with_hw_device():
msg = _("This file is encrypted using a hardware device.") + '\n' \
+ _("Press 'Next' to choose device to decrypt.")
self.wallet_needs_hw_unlock = True
def on_filename(self, filename):
try:
# FIXME? "filename" might contain ".." (etc) and hence sketchy path traversals are possible
nonlocal temp_storage
temp_storage = None
msg = None
self.wallet_exists = False
self.wallet_is_open = False
self.wallet_needs_hw_unlock = False
if filename:
_path = os.path.join(datadir_wallet_folder, filename)
wallet_from_memory = self.wizard._daemon.get_wallet(_path)
try:
if wallet_from_memory:
temp_storage = wallet_from_memory.storage
self.wallet_is_open = True
else:
temp_storage = WalletStorage(_path)
self.wallet_exists = temp_storage.file_exists()
except (StorageReadWriteError, WalletFileException) as e:
msg = _('Cannot read file') + f'\n{str(e)}'
except UnicodeDecodeError:
msg = _('Cannot read file (invalid UTF-8 encoding)')
except Exception as e:
# Prevent OverflowError when error message is too large
if len(str(e)) > 10000: # Reasonable limit for error message
msg = _("Error message too large to display")
else:
msg = _('Cannot read file') + f'\n{str(e)}'
self.logger.exception('')
else:
msg = ""

self.valid = temp_storage is not None
user_needs_to_enter_password = False
if temp_storage:
if not temp_storage.file_exists():
msg = _("This file does not exist.") + '\n' \
+ _("Press 'Next' to create this wallet, or choose another file.")
elif not wallet_from_memory:
if temp_storage.is_encrypted_with_user_pw():
msg = _("This file is encrypted with a password.") + '\n' \
+ _('Enter your password or choose another file.')
user_needs_to_enter_password = True
elif temp_storage.is_encrypted_with_hw_device():
msg = _("This file is encrypted using a hardware device.") + '\n' \
+ _("Press 'Next' to choose device to decrypt.")
self.wallet_needs_hw_unlock = True
else:
msg = _("Press 'Finish' to open this wallet.")
else:
msg = _("Press 'Finish' to open this wallet.")
msg = _("This file is already open in memory.") + "\n" \
+ _("Press 'Finish' to create/focus window.")
if msg is None:
msg = _('Cannot read file')
if filename and os.path.isabs(relative_path(_path)):
msg += '\n\n' + _('Note: this wallet file is outside the default wallets folder.')
msg_label.setText(msg)
widget_create_new.setVisible(bool(temp_storage and temp_storage.file_exists()))
if user_needs_to_enter_password:
pw_label.show()
self.pw_e.show()
if not self.name_e.hasFocus():
self.pw_e.setFocus()
else:
msg = _("This file is already open in memory.") + "\n" \
+ _("Press 'Finish' to create/focus window.")
if msg is None:
msg = _('Cannot read file')
if filename and os.path.isabs(relative_path(_path)):
msg += '\n\n' + _('Note: this wallet file is outside the default wallets folder.')
msg_label.setText(msg)
widget_create_new.setVisible(bool(temp_storage and temp_storage.file_exists()))
if user_needs_to_enter_password:
pw_label.show()
self.pw_e.show()
if not self.name_e.hasFocus():
self.pw_e.setFocus()
else:
pw_label.hide()
self.pw_e.hide()
self.on_updated()

button.clicked.connect(on_choose)
button_create_new.clicked.connect(
lambda: self.name_e.setText(get_new_wallet_name(datadir_wallet_folder))) # FIXME get_new_wallet_name might raise
self.name_e.textChanged.connect(on_filename)
self.name_e.setText(relative_path(path))
pw_label.hide()
self.pw_e.hide()
self.on_updated()
except Exception as e:
if len(str(e)) > 10000:
e = _("Error message too large to display")
self.show_error(str(e))
return

def initialFocus(self) -> Optional[QWidget]:
return self.pw_e
Expand Down
24 changes: 16 additions & 8 deletions electrum/storage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@ class StorageOnDiskUnexpectedlyChanged(Exception): pass

# TODO: Rename to Storage
class WalletStorage(Logger):

# TODO maybe split this into separate create() and open() classmethods, to prevent some bugs.
# Until then, the onus is on the caller to check file_exists().

def __init__(self, path):
Logger.__init__(self)
self.path = standardize_path(path)
Expand All @@ -76,17 +74,27 @@ def __init__(self, path):
except IOError as e:
raise StorageReadWriteError(e) from e
if self.file_exists():
with open(self.path, "rb") as f:
self.raw = f.read().decode("utf-8")
self.pos = f.seek(0, os.SEEK_END)
self.init_pos = self.pos
self._encryption_version = self._init_encryption_version()
try:
with open(self.path, "rb") as f:
self.raw = f.read()
try:
self.raw = self.raw.decode('utf-8')
except UnicodeDecodeError:
raise InvalidWalletError(_("Wallet file contains invalid UTF-8 data. The wallet might be corrupted."))
self.pos = f.seek(0, os.SEEK_END)
self.init_pos = self.pos
self._encryption_version = self._init_encryption_version()
except FileNotFoundError:
raise WalletFileNotFound(path)
except Exception as e:
raise IOError(f"Error opening wallet file: {str(e)}")
else:
self.raw = ''
self._encryption_version = StorageEncryptionVersion.PLAINTEXT
self.pos = 0
self.init_pos = 0


def read(self):
return self.decrypted if self.is_encrypted() else self.raw

Expand Down