Bug#550978: patch incomplete

2010-01-02 Thread Enrico Scholz
Erik Schanze  writes:

>>   
>> http://cvs.fedoraproject.org/viewvc/rpms/gif2png/devel/gif2png-overflow.patch?revision=HEAD&root=extras&view=markup
>> 
>> solves the issue better. 
>
> You're right. Thank you for your attention.

fwiw, I changed my patch to abort/fail when filename length exceeds a
certain size.  That's better than the current strncpy() stuff which
might cause to process a different file than this what was given (and
perhaps validated) by the caller.


>> It omits the changes in processfile() because main() guarantees that
>> 'fname' is short enough.
>> 
> But processfile() will remain insecure.

does not matter... it is called from main() only which does all the
range checking.


> I will adapt my patch with your suggestions.
>
>> FWIW, 2.5.2 *is* affected; the -ENAMETOOLONG comes from the open(2)
>> call.  
>> Applying a modified exploit like
>> 
>>   gif2png `perl -e "print '/' x 1024"`/a
>> 
>> still triggers the issue.
>>
> Not for me:

Adjust the '1024'; on Fedora/RHEL not the buffer overflow caused the
crash but the FORTIFY_SORUCE checks.

$ gif2png `perl -e "print '/' x 1024"`/a
*** buffer overflow detected ***: gif2png terminated
=== Backtrace: =
/lib/libc.so.6(__chk_fail+0x41)[0xb76b31c1]
/lib/libc.so.6(__strcpy_chk+0x43)[0xb76b25e3]
gif2png[0x804aae6]
/lib/libc.so.6(__libc_start_main+0xdc)[0xb75e2e9c]
gif2png[0x8048f21]
=== Memory map: 



Enrico



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org



Bug#550978: patch incomplete

2010-01-01 Thread Enrico Scholz
Hi,

I am the Fedora maintainer of gif2png and think that the supplied patch
is incomplete.  In main(), there is done

| - strcpy(name, argv[i]);
| + strncpy( name, argv[i], sizeof( name ) );
| ...
|   strcat(name, ".gif");

which could still overflow 'name'.  I think that

  
http://cvs.fedoraproject.org/viewvc/rpms/gif2png/devel/gif2png-overflow.patch?revision=HEAD&root=extras&view=markup

solves the issue better. It omits the changes in processfile() because
main() guarantees that 'fname' is short enough.


FWIW, 2.5.2 *is* affected; the -ENAMETOOLONG comes from the open(2)
call.  Applying a modified exploit like

  gif2png `perl -e "print '/' x 1024"`/a

still triggers the issue.



Enrico



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org