Skip to content

Commit

Permalink
Shared unrar code: Avoid false negatives
Browse files Browse the repository at this point in the history
These changes fixes the two cases posted in #5653. At first I thought
some of our own early reject magic introduced the problem, but the
culprit was in pristine ClamAV code. We don't quite share objectives
with them so I'm not sure these are bugs at all in the original context.

ClamAV has changed since though, and its unrar is now in C++ (using the
official free code, I think - just like hashcat does). The changes here
resemble the newer C++ code.
  • Loading branch information
magnumripper committed Jan 27, 2025
1 parent 49c9adc commit 3b92a40
Showing 1 changed file with 17 additions and 4 deletions.
21 changes: 17 additions & 4 deletions src/unrar.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* Extract RAR archives
*
* Modified for JtR, (c) magnum 2012. This code use a memory buffer instead
* Modified for JtR, (c) magnum 2012-2025. This code uses a buffer instead
* of a file handle, and decrypts while reading. It does not store inflated
* data, it just CRC's it. Support for older RAR versions was stripped.
* Autoconf stuff was removed.
Expand Down Expand Up @@ -31,6 +31,8 @@
#include <stdlib.h>
#include <string.h>

#include "common.h"

#include "unrar.h"
#include "unrarppm.h"
#include "unrarvm.h"
Expand Down Expand Up @@ -929,6 +931,17 @@ void rar_unpack_init_data(int solid, unpack_data_t *unpack_data)
unpack_data->unp_crc = 0xffffffff;
}

static MAYBE_INLINE int safe_ppm_decode_char(ppm_data_t *ppm_data, const unsigned char **fd, unpack_data_t *unpack_data)
{
int ch = ppm_decode_char(ppm_data, fd, unpack_data);
rar_dbgmsg("PPM char: %d\n", ch);
if (ch == -1) { // Corrupt PPM data found.
ppm_cleanup(&unpack_data->ppm_data); // Reset possibly corrupt PPM data structures.
unpack_data->unp_block_type = BLOCK_LZ; // Set faster and more fail proof LZ mode.
}
return(ch);
}

int rar_unpack29(const unsigned char *fd, int solid, unpack_data_t *unpack_data)
{
unsigned char ldecode[]={0,1,2,3,4,5,6,7,8,10,12,14,16,20,24,28,
Expand Down Expand Up @@ -972,7 +985,7 @@ int rar_unpack29(const unsigned char *fd, int solid, unpack_data_t *unpack_data)
rar_dbgmsg("UnpPtr = %d\n", unpack_data->unp_ptr);
if (unpack_data->in_addr > unpack_data->read_border) {
if (!rar_unp_read_buf(&fd, unpack_data)) {
retval = 0;
//retval = 0; /* GAVE FALSE NEGATIVES */
break;
}
}
Expand All @@ -986,11 +999,11 @@ int rar_unpack29(const unsigned char *fd, int solid, unpack_data_t *unpack_data)
if (ch == -1) { // Corrupt PPM data found.
ppm_cleanup(&unpack_data->ppm_data); // Reset possibly corrupt PPM data structures.
unpack_data->unp_block_type = BLOCK_LZ; // Set faster and more fail proof LZ mode.
retval = 0;
//retval = 0; /* GAVE FALSE NEGATIVES */
break;
}
if (ch == unpack_data->ppm_esc_char) {
next_ch = ppm_decode_char(&unpack_data->ppm_data, &fd, unpack_data);
next_ch = safe_ppm_decode_char(&unpack_data->ppm_data, &fd, unpack_data);
rar_dbgmsg("PPM next char: %d\n", next_ch);
if (next_ch == -1) { // Corrupt PPM data found.
retval = 0;
Expand Down

0 comments on commit 3b92a40

Please sign in to comment.