Having fought with image rewriting on our platform a little little bit
(some of my colleagues have torn their hair out over it), I know that
this sounds like a good idea. It only sounds like one. There is no
Java image library (JAI, AWT, JNI to ImageMagick, you name it) than
can resist a determined attacker or a dumb user wanting to take your
JVM down. End of story.

Please, at least, add a switch so that one can turn it off. Thanks. At
least I don't plan to let our Shindig installs to go near there and if
I would rewrite images, I would use our painstakely tuned image
processors. Which don't use Java code. I would assume that at least
Paul has similar war stories to share.

    Ciao
        Henning

On Wed, Jan 21, 2009 at 15:52, Louis Ryan <lr...@google.com> wrote:
> So after a fair bit of research and learning about the evils of the Java
> ImageIO library I plan on re-writing the code using Sanselan (
> http://incubator.apache.org/sanselan/site/index.html) which is an Apache
> incubator project for image handling in pure Java. I'm not aware of any
> issues of one incubator project depending on another incubator project (and
> having such a rule would seem very odd)
>
> On Fri, Dec 12, 2008 at 3:15 PM, Brian Eaton <bea...@google.com> wrote:
>
>> This significantly increases the security risk to Shindig deployments.
>>  We definitely should not turn this on by default, and I'm not sure we
>> should do it at all.
>>
>> The problem is that most Java image handling is done in native code
>> for performance reasons, and that image processing has historically
>> been a source of critical vulnerabilities in the JRE:
>>
>> http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-0243
>> http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-2789
>>
>> If a shindig deployment does turn this on, they need to be prepared to
>> patch their JRE on very short notice.  That's not a realistic option
>> for most java shops.
>>
>> I'm going to look around for alternative implementations that are
>> easier to make secure.
>>
>> Cheers,
>> Brian
>>
>> On Fri, Dec 12, 2008 at 2:43 PM,  <louiscr...@gmail.com> wrote:
>> > Reviewers: shindig-dev,
>> >
>> > Description:
>> > This code is the basis for integrating image rewriting into the Shindig
>> > proxy pipeline. This code will try to make reasonable assumptions about
>> > what target image format is most suitable and then serialize to that
>> > format. If the content is smaller than the original then it is used. A
>> > fairly high degree of configurability is added and protections to ensure
>> > that rogue images cannot create exceesively large memory burdens on the
>> > VM
>> >
>> > For sample traffic seen on Orkut this kind of rewriting can yield pretty
>> > substantial reductions in image size. Ill post more thorough results
>> > later.
>> >
>> > Details
>> > GIF - We dont rewrite animated GIFs though we could it is an expensive
>> > process doing frame to frame differencing. We also dont rewrite
>> > transparent GIF currently though we could later with a reasonable
>> > AlphaImageLoader solution for IE6
>> >
>> > PNG - Most PNG writers dont do a good job stripping metadata or
>> > minimizing palette and bit depth, or they use palettes when the image is
>> > small. This will fix most of this. We also attempt to rewrite to JPEG if
>> > the image is opaque and only if it yields a significant improvement ( >
>> > 20%)  over an efficient PNG. JPEG should only provide significant
>> > improvement if the image is photographic in nature.
>> >
>> > JPEG - We strip metadata and allow for configurable compression level.
>> >
>> > Please review this at http://codereview.appspot.com/10841
>> >
>> > Affected files:
>> >  java/common/conf/shindig.properties
>> >
>>  
>> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BaseOptimizer.java
>> >
>>  
>> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizer.java
>> >
>>  
>> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/ImageRewriter.java
>> >
>>  
>> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/ImageUtils.java
>> >
>>  
>> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JPEGOptimizer.java
>> >
>>  
>> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/OptimizerConfig.java
>> >
>>  
>> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/PNGOptimizer.java
>> >
>>  
>> java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/BaseOptimizerTest.java
>> >
>>  
>> java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizerTest.java
>> >
>>  
>> java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/ImageRewriterTest.java
>> >
>>  
>> java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/JPEGOptimizerTest.java
>> >
>>  
>> java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/PNGOptimizerTest.java
>> >
>>  
>> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/animated.gif
>> >
>>  
>> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/inefficient.png
>> >
>>  
>> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/large.gif
>> >
>>  
>> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/large.jpg
>> >
>>  
>> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/rgbawithnoalpha.png
>> >
>>  
>> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/small.jpg
>> >
>>  
>> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/unanimated.gif
>> >  pom.xml
>> >
>> >
>> >
>>
>

Reply via email to