Search code examples
javascripthtmlsecurityxsspenetration-testing

Preventing 'content-sniffing' type vulnerabilities when handling user-uploaded images?


The problem:

I work on an internal tool that allows users to upload images - and then displays those images back to them and others.

It's a Java/Spring application. I have the benefit of only needing to worry about IE11 exactly and Firefox v38+ (Chrome v43+ would be a nice to have)

After first developing the feature, it seems that users can just create a text file like:

 <script>alert("malicious code here!")</script>

and save it as "maliciousImage.jpg" and upload it.

Later, when that image is displayed inside image tags like:

 <img src="blah?imgName=foobar" id="someImageID">

actualImage.jpg displays normally, and maliciousImage.jpg displays as a broken link - and most importantly no malicious content is interpreted!

However If the user right-clicks on this broken link, and clicks 'view image'... bad things happen.

the browser does 'content-sniffing' a concept which is new to me, detects that 'maliciousImage.jpg' is actually a text file, and very kindly renders it as HTML without hesitation. Any script tags are passed to the JavaScript interpreter and, as you can imagine, we don't want this.

What I've tried so far

In short, every possible combination of response headers I can think of to prevent the browser from content-sniffing. All the answers I've found here on stackoverflow, and other docs, imply that setting the content-type header should prevent most browsers from content-sniffing, and setting X-content options should prevent some versions of IE.

I'm setting the x-content-type-options to no sniff, and I'm setting the response content type. The docs I've read lead me to believe this should stop content-sniffing.

response.setHeader("X-Content-Type-Options", "nosniff"); 
response.setContentType("image/jpg");

I'm intercepting the response and these headers are present, but seem to have no effect on how the malicious content is processed...

I've also tried detecting which images are and are not malicious at the point of upload, but I'm quickly realizing this is very much non-trivial...

End goal:

Naturally - any output at all for images that aren't really images (garbled nonsense, an unhandled exception, etc) would be better than executing the text-file as HTML/javascript in the clear, but displaying any malicious HTML as escaped/CDATA'd plain-text would be ideal... though maybe a bit impractical.


Solution

  • So I ended up fixing this problem but forgot to answer my own question:

    Step 1: blocking invalid images

    To get a quick fix out, I simply added some fairly blunt code that checked if an image was actually an image - during upload and before serving it, using the imageio lib:

    import javax.imageio.ImageIO;
    
    //...... 
    
    Image img = attBO.getImage(imgId);
                
    InputStream x = new ByteArrayInputStream(img.getData());
    BufferedImage s;
    try {
        s = ImageIO.read(x);
        s.getWidth();
    } catch (Exception e) {
        throw new myCustomException("Invalid image");
    }
    

    Now, initially i'd hoped that would fix my problem - but in reality it wasn't that simple and just made generating a payload more difficult.

    While this would block:

     <script>alert("malicious code here!")</script>
    

    It's very possible to generate a valid image that's also an XSS payload - just a little more effort....

    Step 2: framework silliness

    It turned out there was an entire post-processing workflow that I'd never touched, that did things such as append tokens to response bodies and use additional frameworks to decorate responses with CSS, headers, footers etc.

    This meant that, although the controller was explicitly returning image/png, it was being grabbed and placed (as bytes) post processing was taking that bytestream, and wrapping it in a header and footer, to form a fully qualified 'view' - this view would always have the 'content-type' text/html and thus was never displayed correctly.

    The crux of this problem was that my controller was directly returning an image, in a RESTful fashion, when the rest of the framework was built to handle controllers returning full fledged views.

    So I had to step through this workflow and create exceptions for the controllers in my code that returned something other than worked in a restful fashion.

    for example with with site-mesh it was just an exclude(as always, simple fix once I understood the problem...):

    <decorators defaultdir="/WEB-INF/decorators">
    <excludes>
        <pattern>*blah.ctl*</pattern>
    </excludes>
    <decorator name="foo" page="myDecorator.jsp">
        <pattern>*</pattern>
    </decorator>
    

    and then some other other bespoke post-invocation interceptors.

    Step 3: Content negotiation

    Now, I finally got the stage where only image bytecode was being served and no review was being specified or explicitly generated.

    A Spring feature called 'content negotiation' kicked in. It tries to reconcile the 'accepts' header of the request, with the 'messageconverters' it has on hand to produce such responses.

    Because spring by default doesn't have a messageconverter to produce image/png responses, it was falling back to text/html - and I was still seeing problems.

    Now, were I using spring 4, I could've simply added the annotation:

    @Produces("image/png")
    

    to my controller - simple fix...

    Step 4: Legacy dependencies

    but because I only had spring 3.0.5 (and couldn't upgrade it) I had to try other things.

    I tried registering new messageconverters but that was a headache or adding a new post-method interceptor to simply change the content-type back to 'image/png' - but that was a hacky headache.

    In the end I just exposed the request/reponse in the controller, and wrote my image directly to the response body - circumventing Spring's content-negotiation altogether

    ....and finally my image was served as an image and displayed as an image - and no injected code was executed!