Skip to content

Commit

Permalink
RAR3 unrar code: Tweaks
Browse files Browse the repository at this point in the history
Add more sanity checks/reject logic, that also helps robustness by
avoiding out of bounds reads.

Also adds a test asserting that the inflated size meets the expections.

See #5653.
  • Loading branch information
magnumripper committed Jan 28, 2025
1 parent 2737437 commit 126b2a4
Showing 1 changed file with 24 additions and 3 deletions.
27 changes: 24 additions & 3 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 @@ -1054,6 +1056,7 @@ int rar_unpack29(const unsigned char *fd, int solid, unpack_data_t *unpack_data)
} else {
number = rar_decode_number(unpack_data, (struct Decode *)&unpack_data->LD);
//rar_dbgmsg("number = %d\n", number);
/* Sanity check added by magnum */
if (number < 0) {
retval = 0;
break;
Expand All @@ -1063,14 +1066,20 @@ int rar_unpack29(const unsigned char *fd, int solid, unpack_data_t *unpack_data)
continue;
}
if (number >= 271) {
/* Sanity check added by magnum (lbits is same size) */
if (number - 271 >= sizeof(ldecode)) {
retval = 0;
break;
}
length = ldecode[number-=271]+3;
if ((bits=lbits[number]) > 0) {
length += rar_getbits(unpack_data) >> (16-bits);
rar_addbits(unpack_data, bits);
}
dist_number = rar_decode_number(unpack_data,
(struct Decode *)&unpack_data->DD);
if (dist_number < 0) {
/* Sanity checks added by magnum (dbits is same size) */
if (dist_number < 0 || dist_number >= sizeof(ddecode)) {
retval = 0;
break;
}
Expand Down Expand Up @@ -1149,7 +1158,8 @@ int rar_unpack29(const unsigned char *fd, int solid, unpack_data_t *unpack_data)

length_number = rar_decode_number(unpack_data,
(struct Decode *)&unpack_data->RD);
if (length_number < 0) {
/* Sanity checks added by magnum (lbits is same size) */
if (length_number < 0 || length_number >= sizeof(ldecode)) {
retval = 0;
break;
}
Expand All @@ -1163,6 +1173,11 @@ int rar_unpack29(const unsigned char *fd, int solid, unpack_data_t *unpack_data)
continue;
}
if (number < 272) {
/* Sanity check added by magnum (sdbits is same size) */
if (number - 263 >= sizeof(sddecode)) {
retval = 0;
break;
}
distance = sddecode[number-=263]+1;
if ((bits = sdbits[number]) > 0) {
distance += rar_getbits(unpack_data) >> (16-bits);
Expand All @@ -1180,6 +1195,12 @@ int rar_unpack29(const unsigned char *fd, int solid, unpack_data_t *unpack_data)
unp_write_buf(unpack_data);
}

/* Added by magnum */
if (retval && unpack_data->written_size != unpack_data->dest_unp_size) {
//rar_dbgmsg("Passed but only wrote %ld of %ld, degrading to FAIL\n", (long)unpack_data->written_size, (long)unpack_data->dest_unp_size);
retval = 0;
}

//rar_dbgmsg("Written size: %ld\n", (long)unpack_data->written_size);
//rar_dbgmsg("True size: %ld\n", (long)unpack_data->true_size);

Expand Down

0 comments on commit 126b2a4

Please sign in to comment.