binref / refinery

High Octane Triage Analysis
Other
618 stars 62 forks source link

xtzip raises when password is a variable #16

Closed targodan closed 1 year ago

targodan commented 1 year ago

Love this tool and currently learning to use it. :heart:

I followed one tutorial linked in the readme and noticed it crashes as soon as you try to decrypt the zip with a variable.

Here's the stack trace:

Traceback (most recent call last):
  File "/home/<redacted>/Tools/refinery/refinery/units/__init__.py", line 1256, in normalized_action
    yield from (x for x in result if x is not None)
  File "/home/<redacted>/Tools/refinery/refinery/units/__init__.py", line 1256, in <genexpr>
    yield from (x for x in result if x is not None)
  File "/home/<redacted>/Tools/refinery/refinery/units/__init__.py", line 681, in <genexpr>
    return (self.labelled(r) for r in result)
  File "/home/<redacted>/Tools/refinery/refinery/units/formats/__init__.py", line 114, in process
    results: List[UnpackResult] = list(self.unpack(data))
  File "/home/<redacted>/Tools/refinery/refinery/units/formats/archive/xtzip.py", line 36, in unpack
    archive.setpassword(self.args.pwd)
  File "/usr/lib/python3.10/zipfile.py", line 1449, in setpassword
    raise TypeError("pwd: expected bytes, got %s" % type(pwd).__name__)
TypeError: pwd: expected bytes, got ByteStringWrapper

There seems to be a typecheck inside the zipfile library code. Might be due to version changes, not sure. Anyway, I fixed it for you. PR incoming. :)

huettenhain commented 1 year ago

I checked the PR, but I think the more robust solution to this is f3f5f8384b57531aee463dbab264b0d458561983, which also adds a regression test for this case. I hope I am not breaking good open source etiquette by rejecting the PR, I am very grateful for the bugreport and the fix!

huettenhain commented 1 year ago

PS: I am releasing a new version that includes this bugfix so the most recent version does not contradict one of the few good articles about refinery ;D. Again, thank you very much for the contribution!

targodan commented 1 year ago

Your welcome! :)

The rejection is not a problem. I agree, your version is probably safer (and has tests). :)