On Wednesday 04 March 2009 00:00:25 Daniel Cheng wrote:
> On Wed, Mar 4, 2009 at 2:33 AM, Matthew Toseland
> <toad at amphibian.dyndns.org> wrote:
> > On Sunday 01 March 2009 13:06:57 j16sdiz at freenetproject.org 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 .. )
Doh, so it is. No problem. :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
URL:
<https://emu.freenetproject.org/pipermail/devl/attachments/20090304/8a83aaba/attachment.pgp>
