xorl %eax, %eax

CVE-2009-1241: ClamAV RAR Parser Security Bypass

leave a comment »

On 2 April 2009, security engineer Thierry Zoller disclosed this vulnerability which allows attackers bypass the ClamAV scanner since RAR unarchiver built-in in ClamAV wasn’t able to handle specific RAR files. Releases prior to 0.95 are vulnerable to this issue. The following code is from ClamAV 0.94.2 release which is the last vulnerable to that bug release, here is the vulnerable function:

1 /*
2  *  Extract RAR archives
152 static void unp_write_data(unpack_data_t *unpack_data, uint8_t *data, int size)
153 {
154         int ret;
155         rar_dbgmsg("in unp_write_data length=%d\n", size);
156         if((ret = write(unpack_data->ofd, data, size)) > 0)
157             unpack_data->written_size += ret;
158         unpack_data->unp_crc = rar_crc(unpack_data->unp_crc, data, size);
159 }

This function is used to write (line 156) data of the provided pointer data (its second argument) to the unpack_data pointer which is the data to be unpacked. This can be found at libclamunrar/unrar.c file. If write() operation succeeds, then its CRC is being updated at line 158. This function is used directly on the user controlled RAR file. To have a better understanding of the above function here are a few members of the unpack_data_t structure as seen at libclamunrar/unrar.h:

187 typedef struct unpack_data_tag
188 {
189         int ofd;
219         int64_t written_size;
220         int64_t dest_unp_size;
221         rarvm_data_t rarvm_data;
222         unsigned int unp_crc;
223         uint32_t pack_size;
238 } unpack_data_t;

A malicious RAR file might lead to a miscalculation at line 157 if its size is really big. To patch this, two new members where added to the above structure like this:

     int64_t written_size;
+    int64_t true_size;
+    int64_t max_size;
     int64_t dest_unp_size;

And those are used to represent the actual size of the file to be unpacked (true_size) and the maximum size that can be processed (max_size). Those where initialized at unp_write_area() like this:

     unpack_data->read_border = 0;
     unpack_data->written_size = 0;
+    unpack_data->true_size = 0;

And at unrar_open() of libclamunrar_iface/unrar_iface.c like this:

     unpack_data->unp_crc = 0xffffffff;
+    unpack_data->max_size = 0;

In addition, a new member was added to the unrar_state_t structure at libclamunrar_iface/unrar_iface.h like this:

     char *comment_dir;
     unsigned long file_count;
+    uint64_t maxfilesize;
     int fd, ofd;     
     char filename[1024];
 } unrar_state_t;

This was later being used at unrar_extract_next() to set the maximum file size which was not being done at all:

442 int unrar_extract_next(unrar_state_t *state, const char *dirname)
449     if(lseek(state->fd, state->file_header->start_offset+state->file_hea    
449 der->head_size, SEEK_SET) != state->file_header->start_offset+state->fil    
449 e_header->head_size) 
467         ofd = open(state->filename, O_RDWR|O_CREAT|O_TRUNC|O_BINARY, 0600);
474         unpack_data = (unpack_data_t *) state->unpack_data;
475         state->ofd = unpack_data->ofd = ofd;
476         if(state->file_header->method == 0x30) {

Now, maximum file size (as well as the max_size) is initialized here like this:

     state->ofd = unpack_data->ofd = ofd;
+    unpack_data->max_size = state->maxfilesize;
     if(state->file_header->method == 0x30) {

At last, unp_write_data() has now the required additional checks which are:

     rar_dbgmsg("in unp_write_data length=%d\n", size);
+    unpack_data->true_size += size;
+    unpack_data->unp_crc = rar_crc(unpack_data->unp_crc, data, size);
+    if(unpack_data->max_size) {
+        if(unpack_data->written_size >= unpack_data->max_size)
+        return;
+        if(unpack_data->written_size + size > unpack_data->max_size)
+        size = unpack_data->max_size - unpack_data->written_size;
+    }
     if((ret = write(unpack_data->ofd, data, size)) > 0)
         unpack_data->written_size += ret;
-    unpack_data->unp_crc = rar_crc(unpack_data->unp_crc, data, size);

The true_size is updated and the CRC is calculated, then a check is being perfomed at the maximum size and if this is not equal to zero and the written_size less than or equal to the maximum size it will immediately return. Else, if the maximum size is not zero but there is still space for writing data, another check is made to ensure that the written_size + size will not result in a number greater than the maximum size.
The final patch was at libclamav/scanners.c where function cli_scanrar() is located. The patch was simply to include the new features in the initial checks:

-    if((ret=cli_checklimits("RAR", ctx, rar_state.metadata_tail->unpack_size, rar_state.metadata_tail->pack_size, 0)!=CL_CLEAN)) {
+    if(ctx->limits && ctx->limits->maxscansize && ctx->scansize >= ctx->limits->maxscansize) {
         ret = CL_CLEAN;
-        continue;
+        break;
+    if(ctx->limits && ctx->limits->maxscansize && ctx->scansize + ctx->limits->maxfilesize >= ctx->limits->maxscansize)
+        rar_state.maxfilesize = ctx->limits->maxscansize - ctx->scansize;
+    else
+        rar_state.maxfilesize = ctx->limits ? ctx->limits->maxfilesize : 0;
     ret = unrar_extract_next(&rar_state,dir);

This patch was made by Tomasz Kojm on 23 March 2009.

By the way, did anyone notice that on ClamAV’s security category on their site hasn’t been updated since November 2008! Have a look… Do you have any idea why they’re doing this?

Written by xorl

April 22, 2009 at 11:37

Posted in bugs

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s