On Wednesday 04 March 2009 00:00:25 Daniel Cheng wrote: > 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 .. )
Doh, so it is. No problem. :)
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Devl mailing list [email protected] http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
