Godzil / theunarchiver

Automatically exported from code.google.com/p/theunarchiver
Other
0 stars 0 forks source link

XADCFBFParser memory management issues #690

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
There seem to be various bugs in XADCFBFParser, such as uninitialized 
variables, forgotten frees or undocumented call order dependency (parse needed 
to be called before any decompression)

diff -r d813330443dd XADMaster/XADCFBFParser.m
--- a/XADMaster/XADCFBFParser.m Fri Jun 14 01:01:16 2013 +0300
+++ b/XADMaster/XADCFBFParser.m Sun Aug 11 10:51:21 2013 +0200
@@ -9,6 +9,7 @@
    if((self=[super init]))
    {
        sectable=NULL;
+       minisectable=NULL;
    }
    return self;
 }
@@ -16,6 +17,7 @@
 -(void)dealloc
 {
    free(sectable);
+   free(minisectable);
    [super dealloc];
 }

@@ -60,6 +62,8 @@
    int idspersec=secsize/4;

    numsectors=numtablesecs*idspersec;
+   if (sectable)
+       free(sectable);
    sectable=malloc(numsectors*sizeof(uint32_t));

    for(int i=0;i<numtablesecs;i++)
@@ -87,6 +91,8 @@
    // Read short-sector allocation table

    numminisectors=numminitablesecs*idspersec;
+   if (minisectable)
+       free(minisectable);
    minisectable=malloc(numminisectors*sizeof(uint32_t));

    uint32_t minitablesec=firstminitablesec;
@@ -227,6 +233,9 @@

 -(void)processEntry:(uint32_t)n atPath:(XADPath *)path entries:(NSArray *)entries
 {
+   if (sectable == NULL)
+       [self parse];
+
    NSMutableDictionary *entry=[entries objectAtIndex:n];

    uint32_t left=[[entry objectForKey:@"CFBFLeftChild"] unsignedIntValue];
@@ -263,6 +272,9 @@

 -(CSHandle *)handleForEntryWithDictionary:(NSDictionary *)dict wantChecksum:(BOOL)checksum
 {
+   if (sectable == NULL)
+       [self parse];
+
    CSHandle *handle=[self handle];
    off_t size=[[dict objectForKey:XADFileSizeKey] longLongValue];
    uint32_t first=[[dict objectForKey:@"CFBFFirstSector"] unsignedIntValue];

Original issue reported on code.google.com by gerh...@disneyresearch.com on 11 Aug 2013 at 8:54

GoogleCodeExporter commented 8 years ago
I'll look into it, but parse needs to be called before any decompression for 
every decompressor, so that is entirely normal.

Original comment by paracel...@gmail.com on 11 Aug 2013 at 9:08

GoogleCodeExporter commented 8 years ago
That's inconvenient :/

Just storing the description dictionaries seems to be enough for zip/rar. What 
I do in my program is parse lots of files (more than I can have open at the 
same time due to file descriptor limitations), and keep the description dicts. 
Then at a later time I open the archive file again and decompress a containe 
file.

Anyway, the other problems are mainly related with missing calling free for 
memory malloced.

Original comment by gerh...@disneyresearch.com on 11 Aug 2013 at 9:42

GoogleCodeExporter commented 8 years ago
That usage works for a few parsers, but it is not guaranteed to work. You 
should probably use a whitelist of formats for which it works. Some formats 
require more information to be extracted from the file format than can be 
stored in the entry dicts.

Original comment by paracel...@gmail.com on 11 Aug 2013 at 9:57

GoogleCodeExporter commented 8 years ago
Fixed the missing free.

Original comment by paracel...@gmail.com on 6 Sep 2013 at 2:01