Re: [sleuthkit-developers] sleuthkit bug in fs_data.c

[David C. <da...@in...>]

Hi Brian,

On Fri, Nov 17, 2006 at 05:13:28PM -0500, Brian Carrier wrote:
> Indeed you are correct.  I just fixed it (the next version will 
> hopefully be out next week once I track down an elusive NTFS compression 
> bug).

I think I've seen this too, on friday afternoon, I didnt get much time
to look into it. glibc detecting a double free in ntfs_uncompress_done??

Let me know this week if you want me to test your solution.

> Where else did you see this issue?  I think fs_data is the only 
> structure that is reused.

If you grep the sources in src/fstools for FS_DATA_INUSE, you will see a few
more while loops with the same structure e.g.:

...
dcalc_lib.c:151:        while ((fs_data) && (fs_data->flags & FS_DATA_INUSE)) {
dls_lib.c:178:  while ((fs_data) && (fs_data->flags & FS_DATA_INUSE)) {
fls_lib.c:137:      while ((fs_data) && (fs_data->flags & FS_DATA_INUSE)) {
...
(there are more)

These loops will also quit early if they see a hole right? If your
solution was to change fs_data_getnew_attr, then they dont matter, but
if you used my patch, these loops will have to be fixed too.

Thanks,
Dave

> David Collett wrote:
> >G'Day Brian,
> >
> >I believe I have found a bug in sleuthkit.
> >
> >in fs_data.c:
> >
> >fs_data_getnew_attr can create unused 'holes' in attr lists. At least
> >two functions, fs_data_lookup and fs_data_lookup_noid do not jump the
> >holes which can cause them to return early without finding the correct
> >FS_DATA element to return.
> > 
> >This does not seem to effect sleuthkit (though I havent done much
> >testing), but it does effect pyflag. Probably because we live for longer
> >and perform more walks etc than the sk tools. As such there is more
> >opportunities for structure reuse and hence more chances to hit this
> >bug.
> > 
> >My solution for now is to change the loops in fs_data_lookup and
> >fs_data_lookup_noid to jump the holes (patch attached), though another
> >possibility is to modify fs_data_getnew_attr so that it does not create
> >holes in the first place. This may be more complicated and undesirable
> >though.
> >
> >My patch *only* fixes the loops in fs_data. A quick grep through the
> >code reveals several more similar loops which could cause trouble
> >(though through luck, they probably dont!).
> >
> >Let me know what you think or if you need a better description of the
> >problem, I can fix up the rest for you and send another patch if you
> >like.
> >
> >Thanks,
> >David Collett
> >
> >
> >------------------------------------------------------------------------
> >
> >diff -ruN sleuthkit-2.06/src/fstools/fs_data.c 
> >sleuthkit-2.06-dave/src/fstools/fs_data.c
> >--- sleuthkit-2.06/src/fstools/fs_data.c	2006-09-02 
> >02:09:15.000000000 +1000
> >+++ sleuthkit-2.06-dave/src/fstools/fs_data.c	2006-11-17 
> >18:10:59.615647500 +1100
> >@@ -242,9 +242,11 @@
> > 	return NULL;
> >     }
> > 
> >-    while ((fs_data) && (fs_data->flags & FS_DATA_INUSE) &&
> >-	((fs_data->type != type) || (fs_data->id != id)))
> >-	fs_data = fs_data->next;
> >+    while (fs_data) {
> >+        if((fs_data->flags & FS_DATA_INUSE) && (fs_data->type == type) && 
> >(fs_data->id == id))
> >+            break;
> >+        fs_data = fs_data->next;
> >+    }
> > 
> >     if ((!fs_data) || (fs_data->type != type) || (fs_data->id != id)) {
> > 	return NULL;
> >@@ -280,8 +282,8 @@
> >      * lowest id of the given type (if more than one exists) 
> >      */
> > 
> >-    while ((fs_data) && (fs_data->flags & FS_DATA_INUSE)) {
> >-	if (fs_data->type == type) {
> >+    while (fs_data) {
> >+	if ((fs_data->flags & FS_DATA_INUSE) && fs_data->type == type) {
> > 
> > 	    /* replace existing if new is lower */
> > 	    if ((!fs_data_ret) || (fs_data_ret->id > fs_data->id))
> >
> >
> >------------------------------------------------------------------------
> >
> >-------------------------------------------------------------------------
> >Take Surveys. Earn Cash. Influence the Future of IT
> >Join SourceForge.net's Techsay panel and you'll get the chance to share 
> >your
> >opinions on IT & business topics through brief surveys - and earn cash
> >http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
> >
> >
> >------------------------------------------------------------------------
> >
> >_______________________________________________
> >sleuthkit-developers mailing list
> >sle...@li...
> >https://lists.sourceforge.net/lists/listinfo/sleuthkit-developers