Skip to content
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

Warn if a pre RAR 3 file is detected #5273

Closed
wants to merge 1 commit into from

Conversation

claudioandre-br
Copy link
Member

Closes #5271

Closes the bug, we can keep the issue open as an "enhancement" to request to implement the "new format".

We still cannot handle these files, so bail out and emit a warning.
Copy link
Member

@magnumripper magnumripper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I came up with a slightly different patch before I saw this PR:

diff --git a/src/rar2john.c b/src/rar2john.c
index c7a4ae78b..290ba18d0 100644
--- a/src/rar2john.c
+++ b/src/rar2john.c
@@ -562,6 +562,10 @@ next_file_header:
 			fprintf(stderr, "! not encrypted, skipping\n");
 			jtr_fseek64(fp, file_hdr_pack_size, SEEK_CUR);
 			goto next_file_header;
+		} else if (file_hdr_block[24] < 29) {
+			fprintf(stderr, "! %s: Too old RAR file version (v%0.1f encryption), not supported.\n",
+			        archive_name, (float)file_hdr_block[24] / 10.);
+			goto err;
 		}
 
 		method = file_hdr_block[25];

The reason I chose v2.9 is our unpack code is called rar_unpack29(). I tested the above patch against our samples and it seems fine - but I'm not sure mine is better or worse.

@claudioandre-br
Copy link
Member Author

I was confused whether to go to err or BailOut. anyway, Please feel free to choose

@magnumripper
Copy link
Member

I'll do some more testing real quick.

@magnumripper
Copy link
Member

OK so yours and mine version handle all samples the same. I'm not quite sure but I think we should go with my version - I'm pretty sure the version (archive format version, not RAR version) where it changed was v2.9.

I'll make a PR with that, then let's sleep on it.

@magnumripper
Copy link
Member

I was confused whether to go to err or BailOut. anyway, Please feel free to choose

I amended my patch to do neither - it now skips to next file in the archive. In the (remote) case there are a mix of format versions in the archive, we might be able to handle some later file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RAR3-p hash with *35 ending won't find password in wordlist
2 participants