On Wed, Mar 4, 2009 at 2:33 AM, Matthew Toseland
<[email protected]> wrote:
> On Sunday 01 March 2009 13:06:57 [email protected] wrote:
>> Author: j16sdiz
>> Date: 2009-03-01 13:06:56 +0000 (Sun, 01 Mar 2009)
>> New Revision: 25867
>>
>> Modified:
>>    trunk/freenet/src/freenet/clients/http/filter/PNGFilter.java
>> Log:
>> PNGFilter should throw DataFilterException instead of IOException
>>
>> use a throwError() function, just like JPEGFilter do.
>>
>> Modified: trunk/freenet/src/freenet/clients/http/filter/PNGFilter.java
>> ===================================================================
>> --- trunk/freenet/src/freenet/clients/http/filter/PNGFilter.java      
>> 2009-03-01
> 07:25:00 UTC (rev 25866)
>> +++ trunk/freenet/src/freenet/clients/http/filter/PNGFilter.java      
>> 2009-03-01
> 13:06:56 UTC (rev 25867)
>> @@ -160,8 +160,10 @@
>>                                       if((val >= 65 && val <= 99) || (val >= 
>> 97 && val <= 122)) {
>>                                               chunkTypeBytes[i] = 
>> lengthBytes[i];
>>                                               sb.append(val);
>> -                                     } else
>> -                                             throw new IOException("The 
>> name of the chunk is invalid!");
>> +                                     } else {
>> +                                             String chunkName = 
>> HexUtil.bytesToHex(lengthBytes, 0, 4);
>> +                                             throwError("Unknown Chunk"  , 
>> "The name of the chunk is invalid! (" +
> chunkName+")");
>> +                                     }
>>                               }
>>                               chunkTypeString = sb.toString();
>>                               if(logMINOR)
>> @@ -204,30 +206,30 @@
>>
>>                               if(!skip && "IHDR".equals(chunkTypeString)) {
>>                                       if(hasSeenIHDR)
>> -                                             throw new IOException("Two 
>> IHDR chunks detected!!");
>> +                                             throwError("Duplicate IHDR", 
>> "Two IHDR chunks detected!!");
>>                                       hasSeenIHDR = true;
>>                                       validChunkType = true;
>>                               }
>>
>>                               if(!hasSeenIHDR)
>> -                                     throw new IOException("No IHDR 
>> chunk!");
>> +                                     throwError("No IHDR chunk!", "No IHDR 
>> chunk!");
>>
>>                               if(!skip && "IEND".equals(chunkTypeString)) {
>> -                                     if(hasSeenIEND)
>> -                                             throw new IOException("Two 
>> IEND chunks detected!!");
>> +                                     if(hasSeenIEND) // XXX impossible code 
>> path: it should have throwed
> as "IEND not last chunk"
>> +                                             throwError("Two IEND chunks 
>> detected!!", "Two IEND chunks
> detected!!");
>>                                       hasSeenIEND = true;
>>                                       validChunkType = true;
>>                               }
>>
>>                               if(!skip && 
>> "PLTE".equalsIgnoreCase(chunkTypeString)) {
>>                                       if(hasSeenIDAT)
>> -                                             throw new IOException("PLTE 
>> must be before IDAT");
>> +                                             throwError("PLTE must be 
>> before IDAT", "PLTE must be before IDAT");
>>                                       validChunkType = true;
>>                               }
>>
>>                               if(!skip && 
>> "IDAT".equalsIgnoreCase(chunkTypeString)) {
>>                                       if(hasSeenIDAT && 
>> !"IDAT".equalsIgnoreCase(lastChunkType))
>> -                                             throw new 
>> IOException("Multiple IDAT chunks must be consecutive!");
>> +                                             throwError("Multiple IDAT 
>> chunks must be consecutive!", "Multiple
> IDAT chunks must be consecutive!");
>>                                       hasSeenIDAT = true;
>>                                       validChunkType = true;
>>                               }
>> @@ -241,7 +243,7 @@
>>
>>                               if(dis.available() < 1) {
>>                                       if(!(hasSeenIEND && hasSeenIHDR))
>> -                                             throw new IOException("Missing 
>> IEND or IHDR!");
>> +                                             throwError("Missing IEND or 
>> IHDR!", "Missing IEND or IHDR!");
>>                                       finished = true;
>>                               }
>>
>> @@ -273,7 +275,7 @@
>>                               lastChunkType = chunkTypeString;
>>                       }
>>                       if(hasSeenIEND && dis.available() > 0 && output == 
>> null)
>> -                             throw new IOException("IEND not last chunk");
>> +                             throwError("IEND not last chunk", "IEND not 
>> last chunk");
>>
>>                       dis.close();
>>               } finally {
>> @@ -313,4 +315,18 @@
>>                       data.free();
>>               }
>>       }
>> +
>> +     private void throwError(String shortReason, String reason) throws
> DataFilterException {
>> +             // Throw an exception
>> +             String message = "Invalid PNG";
>> +             if(reason != null)
>> +                     message += ' ' + reason;
>> +             if(shortReason != null)
>> +                     message += " - " + shortReason;
>> +             DataFilterException e = new DataFilterException(shortReason, 
>> shortReason,
>> +                             "<p>"+message+"</p>", new 
>> HTMLNode("p").addChild("#", message));
>> +             if(Logger.shouldLog(Logger.NORMAL, this))
>> +                     Logger.normal(this, "Throwing "+e, e);
>> +             throw e;
>> +     }
>>  }
>
> Congratulations, you've added an exploitable bug. :|
>
> The message is not filtered in throwError(), therefore putting unfiltered text
> into it from the chunk name is potentially exploitable.

Ugh?
All message in throwError are static message.... no text from the
original png file.
(except the first one ... but that is a hex dump using HexUtil ..   )
_______________________________________________
Devl mailing list
[email protected]
http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl

Reply via email to