Spring Security AbstractRequestParameterAllowFromStrategyTests and intern WhiteListedAllowFromStrategy uses querystring parameters

If you have configured

WhiteListedAllowFromStrategy w = new WhiteListedAllowFromStrategy(
                    Arrays.asList("http://localhost:3000",
                            "http://localhost:9082",
                            "somethingthatiswhitelisted"));
            wlafs.setAllowFromParameterName("x-frames-allow-from"); // I know default value
            http.headers().frameOptions().disable();
            http.headers().addHeaderWriter(new XFrameOptionsHeaderWriter(w));

The code that is eventually called to see if the calling website should be able to frame it is:

public String getAllowFromValue(HttpServletRequest request) {
        String allowFromOrigin = request.getParameter(this.allowFromParameterName);
        if (this.log.isDebugEnabled()) {
            this.log.debug("Supplied origin '" + allowFromOrigin + "'");
        }

        return StringUtils.hasText(allowFromOrigin) && this.allowed(allowFromOrigin) ? allowFromOrigin : "DENY";
    }

Where as request.getParameter is set via query string from the calling website... that is if I know what you have set in your white list I can simply make my IFrame have: <iframe src="http://somewebsite?x-frames-allow-from=somethingthatiswhitelisted"></iframe>

This seems to be pretty weak protection, would it not be be better to use something like: request.getHeader("referer") vs a request.getParameter that can easily be set by the calling website that wished to embed your content? Using the header("referer") would also just work vs having to manually set a parameter, and or guard against someone artificially setting the parameter via a query string.

Comment From: jchambers-ln

Actually probably more accurately should add a new class RefererWhiteListedAllowFromStrategy

Something like:

package org.springframework.security.web.header.writers.frameoption;

import org.springframework.security.web.header.writers.frameoptions.AllowFromStrategy;
import java.util.Collection;

import org.springframework.util.Assert;

import javax.servlet.http.HttpServletRequest;

public class RefererWhiteListedAllowFromStrategy implements AllowFromStrategy  {
    private final Collection<String> allowed;

    /**
     * Creates a new instance
     * @param allowed the origins that are allowed.
     */
    public RefererWhiteListedAllowFromStrategy (Collection<String> allowed) {
        Assert.notEmpty(allowed, "Allowed origins cannot be empty.");
        this.allowed = allowed;
    }

    @Override
    public String getAllowFromValue(HttpServletRequest request) {
       //todo: harden for trailing slashes and different protocols http/https 
        if(allowed.contains(request.getHeader("referer"))){
            return request.getHeader("referer");
        }
        return "DENY";
    }
}

Then it can be used with something like: (note request.getHeader("referer"); typically has the trailing slash, so for hardened code probably would want to make the code handle with or without trailing slash.)

RefererWhiteListedAllowFromStrategy w= new RefererWhiteListedAllowFromStrategy (
                    Arrays.asList("http://localhost:3000/",
                            "http://localhost:9082/",
                            "http://localhost:8088/"));
            //wlafs.setAllowFromParameterName("referer");
            http.headers().frameOptions().disable();
            http.headers().addHeaderWriter(new XFrameOptionsHeaderWriter(
                    w
                    ));

For now I've just done something similar with a local class to meet my needs. I know referer can be spoofed by the client, but more worried about a website embedding content than an individual user tweaking the header here.

Comment From: fhanik

hi @jchambers-ln

Looking through the history and some stackoverflow, this works as designed.

As you mention, it's not really a protection scheme, and you have a valid workaround. I'm going to close this categorized as not and issue.

If you feel strongly about making the case for an enhancement, I would open up a PR with test cases. Please note, you'll want to take a look at HeadersBeanDefinitionParser.java too, as this component touches on the same logic.