-
Notifications
You must be signed in to change notification settings - Fork 4
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.
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
Device freezing if "1" was selected as shares threshold #36
Comments
Thanks again for this issue report. This does not happen when testing with Speculos on a Ledger Nano S+ but I shall investigate further to see what the difference is between Speculos and a physical device. I will also test on a Ledger Nano S to see if there are differences between devices. |
Further investigation reveals that this issue does in fact happen when testing with Speculos. It also seems to occur on all devices. This will make testing and troubleshooting much much easier and quicker. The problem is most likely to be some edge case that needs to be caught. But we'll see. |
Problem found. The cause of the issue is this error check: Lines 193 to 194 in 6a841cf
The
I now need to check if the SSKR standard actually allows a threshold of 1 when share count is greater than 1. If not then the option should be disabled in the app. Disabled as opposed to throwing an error as it is currently doing. A quick look at the SSKR standard documentation does not mention this condition but for security reasons it makes sense that such a condition may exist. |
seedtool-cli confirms that 1-of-2 is not allowed: aido@dev$ seedtool --in hex 59f2293a5bce7d4de59e71b4207ac5d2 --out sskr --group=2-of-3
tuna acid epic gyro free able able acid able race iris play yoga good lamb solo apex cook inch wave quad tent hang wall yurt many nail deli chef
tuna acid epic gyro free able able acid acid gear wall body apex meow miss gift wand jowl bulb dark void keys keno tiny math item gear fizz away
tuna acid epic gyro free able able acid also grim jugs sets buzz scar quiz twin song zest list idle claw monk buzz menu down list buzz cost soap
aido@dev$
aido@dev$ seedtool --in hex 59f2293a5bce7d4de59e71b4207ac5d2 --out sskr --group=1-of-1
tuna acid epic gyro soap mild able able able hawk whiz diet fact help taco kiwi gift view noon jugs quiz crux kiln silk tied brew cusp noon wasp
aido@dev$
aido@dev$ seedtool --in hex 59f2293a5bce7d4de59e71b4207ac5d2 --out sskr --group=1-of-2
seedtool: Invalid group specifier. 1-of-M groups where M > 1 are not supported.
Try `seedtool --help' or `seedtool --usage' for more information. |
Hi @DmKoshelek, The latest commit fixes this issue and it will be included in v1.7.4. Instead of ungracefully freezing the app now gracefully provides a warning if a user chooses 1-of-m shares when m > 1. |
Today during testing #32 issue I find out that I can select "1" as a number of thresholds. If you think about that it is the same as sharing whole seed phrase, but I decided to try to generate shares anyway. And device have just been frozen.
Steps to reproduce the behavior:
Actual result: Device is frozen
Expected result: You can't select "1" as a threshold
Device: Ledger Nano S+
Firmware version: 1.1.1
App Version v1.7.4-rc.2
The text was updated successfully, but these errors were encountered: