search

gurnec / btcrecover

An open source Bitcoin wallet password and seed recovery tool designed for the case where you already know most of your password/seed, but need assistance in trying different possible combinations.
GNU General Public License v2.0
1.28k stars 686 forks source link

Permit --autosave option when using a --passwordlist #144

Open timmygee opened 6 years ago

timmygee commented 6 years ago

I'm getting the error running:

./btcrecover.py --restore=autosave.txt

The problem is, the passwordlist option IS in the restored autosave file

# cat autosave.txt
�}q(UskipqI15065342897
Uordering_versionqU0.6.4qUargvq]q(U--passwordlist=passwordlist.txtqU--has-wildcardsU--no-dupchecksq     U--autosave=autosave.txtq
U--wallet=wallet.aes.jsonq
                          --no-etaqeu.�}q(UskipqI15050336434
Uordering_versionqU0.6.4qUargvq]q(U--passwordlist=passwordlist.txtqU--has-wildcardsU--no-dupchecksq     U--autosave=autosave.txtq
U--wallet=wallet.aes.jsonq
                          --no-etaqeu.
timmygee commented 6 years ago

Edit: I have identified the issue and have submitted PR #145

Additionally I have 2 other PRs there which were changes I needed to make to get btcrecover working with autosave and a really old blockchain.info wallet. I think the changes may be useful to others so please merge if you're happy to include my changes

gurnec commented 6 years ago

Thanks for your PRs!

I just merged one of them, but I want to hold off on the others for a little bit.

My intention was to not allow at all the --autosave and --passwordlist options at the same time, at least at the moment. I (mostly) go to efforts to prevent someone from changing something (like changing a tokens file) between an initial run and a restored run to prevent people from shooting themselves in the foot, and I hadn't implemented that for passwordlist files yet, so I just lazily disallowed them altogether... or so I thought.

There's clearly something wrong here that I must have missed.

timmygee commented 6 years ago

@gurnec thanks for the merge!

It does seem strange to have two different argument parsing conditions depending on what is specified on the command line. Perhaps a refactor to just a single parse operation will put things on the right path?