Closed jhhcs closed 1 year ago
Hi there,
First, thanks for using the library and bringing this up.
I understand that you were previously (see #81) outspoken against an option for "relaxed" parsing. Have you considered, instead, to use the
logging
module to display warnings that come up during parsing, and to only throw an exception when it is absolutely impossible to recover? I think this would be a good solution as warnings do not remain invisible this way, but if the library can recover, it does not prevent extraction of the files either.
Yes, I am very agreeable to this. In point of fact, my strategy up until this point has been to relax the parsing requirements as people have run into them. I'd say that there are currently 3 different "levels" of parsing errors currently:
For errors of type 1, there's really nothing we can do; if essential metadata is wrong or missing, we have to give up. For errors of type 2, we currently always throw an exception, but we could log an error and attempt to go on. For errors of type 3, we could either ignore the problem completely (though this might indicate a corrupt ISO), or we could log an error and go on.
I'd be happy to relax things for errors of type 2 and type 3, though the devil is going to be in the details of which exceptions are of which of the three types.
PS: I would be happy to contribute with a pull request if you can generally agree to the above strategy.
That would be great! That would provide us a place to discuss whether each one is of type 1, 2, or 3 (and it might even make sense to extend the PycdlibException classes to formally encode that distinction). Thanks for bringing this up.
Fantastic, thanks for the response. I'll dive into the code and I hope to have some pull requests for you soon. I'll probably open a separate PR for each individual "relaxation" and use this issue to track our progress from more of a bids-eye view.
I finally sat down to do some of this. I have a question: Would you agree to having the test samples I have procured added to the repository using git-lfs? I would like to do this test-based, i.e. add one test for each outlier and then provide a suggested adjustment, and then do one pull request for each of those.
PS: Since they are potential malware samples, I would store them in a quarantine format (XOR-encrypted with a fixed key and compressed).
PS: Since they are potential malware samples, I would store them in a quarantine format (XOR-encrypted with a fixed key and compressed).
I really don't want to be storing and then providing malware-laden ISOs anywhere. What I'd rather do here is to look at the changes you are proposing, and then specially hand-craft ISO files that trigger those problems. Usually I do this kind of thing in the tests by generating a valid ISO, then going in and modifying a few bytes as necessary (something like https://github.com/clalancette/pycdlib/blob/5b341b35a5846a24349227c04cd6e81561dee3fc/tests/integration/test_parse.py#L1255-L1274).
I understand. Unfortunately, it is beyond my understanding of the ISO format to do this, and on my fork I still have 8 samples where I don't even have any idea how to achieve successful extraction 😓.
Given that we have merged at least one fix for this issue, and the age of it, I'm going to close this out. Do feel free to open new issues/pull requests for anything else that you'd like to attempt to relax, and I'll be happy to look at it.
Absolutely, that makes sense. I haven't found enough time to look into it further, and if I do, I will create separate issues.
This is related to #3, #81, and #76. I am using this (fantastic, might I say) library in malware research to unpack ISO files that contain malicious samples. It works extremely well. Every now and then, however, I run into an ISO file that can be unpacked without errors using 7zip, but pycdlib throws an exception. I have collected a few samples that illustrate each of the different exceptions I have run into so far:
errors.zip (password:
infected
) Note that this archive contains ISO files that very likely contain malware. Please handle accordingly.The following is a test script that does everything I also do in production; it expects the above archive to be extracted to a folder called "errors" in its the current working directory. It is supposed to list the names of all files within each ISO together with some timestamp information.
With the samples from the above archive and version 1.12.0 of pycdlib, the output is the following:
I understand that you were previously (see #81) outspoken against an option for "relaxed" parsing. Have you considered, instead, to use the
logging
module to display warnings that come up during parsing, and to only throw an exception when it is absolutely impossible to recover? I think this would be a good solution as warnings do not remain invisible this way, but if the library can recover, it does not prevent extraction of the files either.PS: I would be happy to contribute with a pull request if you can generally agree to the above strategy.