Flash scope cookie enables data exploits

ren-zhijun-oracle commented 13 years ago

The implementation of the flash scope in Mojarra 2.x is based on a cookie that contains an index into an application scoped map. This index is a simple integer. New values are obtained by the implementation by incrementing an AtomicInteger:

**ELFlash.java**private long getNewSequenceNumber() {
    long result = sequenceNumber.incrementAndGet();

    if (0 == result % numberOfFlashesBetweenFlashReapings) {
        reapFlashes();
    }

    if (result == Long.MAX_VALUE) {
        result = 1;
        sequenceNumber.set(1);
    }

    return result;
}

This algorithm makes the index very predictable. Since the map that contains all flash data is application scoped, an attacker only needs to known an index in order to access data from random other users. As mentioned this index is very easy to guess. One scheme would be to observe the index being handed out for a particular request (say 1500), then take a slightly higher number (say 2000) and continuously poll the server using this cookie. Eventually this number will be generated server side and it's possible the attacker's request will reach the server before that of the legitimate user.

This exploit is even more severe since all current Mojarra implementations (2.0.x and 2.1.x) contain other bugs that prevent the flash scope from being immediately expired (see http://java.net/jira/browse/JAVASERVERFACES-1751). The user has to do a couple of post backs before this expiration actually happens. This means in the current situation there's a very wide window where the attacker can access data with a guessed index.

Since the Flash scope can contain any kind of data (like e.g. personal information, credit card details, messages, etc) this seems be a rather severe problem.

One solution would be to use a GUID instead of a sequential number. Another solution could be to (optionally) store flash data in the HTTP session and thus piggyback on the security of the JSESSIONID.

Affected Versions

[2.1.1]

ren-zhijun-oracle commented 6 years ago

Issue Imported From: https://github.com/javaserverfaces/mojarra/issues/2130
Original Issue Raised By:@javaserverfaces
Original Issue Assigned To: @edburns
Original Issue Closed By:@javaserverfaces

ren-zhijun-oracle commented 13 years ago

@javaserverfaces Commented Reported by @arjantijms

ren-zhijun-oracle commented 12 years ago

@javaserverfaces Commented frederickkaempfer said: This bug is definitely exploitable, because ELFlash.java uses the unfiltered sequence number from the cookie csfcfc to retrieve the "session"'s flash map. The danger of data theft using flash cookies is very high so the Flash scope should NOT be used in production with sensitive data until this is resolved.

ELFlash does not use any session related information to generate the cookie's key nor when retrieving it. So there is basically no real security check and additionally the sequence numbers are very predictable.

– By the way the spec also mentions that the flash values should never leave context of the user session:

The Flash provides a way to pass temporary objects between the user views generated by the faces lifecycle. Anything one places in the flash will be exposed to the next view encountered by the same user session and then cleared out.

To test create a simple managed bean that accesses the Flash scope:

@ManagedBean
@RequestScoped
public class FlashBean {

    public String getValue() {
        // put the current session id in the context to verify that
        // this value leaks between browser sessions
        Flash flash = FacesContext.getCurrentInstance().getExternalContext().getFlash();

        String sessionId = (String) flash.get("sessionId");

        return sessionId;
    }

    public void createFlash() {
        Flash flash = FacesContext.getCurrentInstance().getExternalContext().getFlash();
        HttpSession session = (HttpSession) FacesContext.getCurrentInstance().getExternalContext().getSession(true);

        String sessionId = session.getId();
        flash.put("sessionId", sessionId);
    }
}

Use this form on a facelet page:

<h:form>
            <h:outputText value="#{flashBean.value}"/>
            <h:commandButton value="Create flash" action="#{flashBean.createFlash()}">
<f:ajax render="@form"/>
            </h:commandButton>
        </h:form>

Now play around with the cookie values in two separate browsers. For example use the csfcfc value sent to one browser when clicking the button. With firebug you should be able to retrieve one browser's session session id in another browser with a different user session.

So as arjan tijms stated, either use GUIDs or encode the current session id in the inner flash map's key.

ren-zhijun-oracle commented 12 years ago

@javaserverfaces Commented frederickkaempfer said: Note that the demo probably only yields results on Mojarra versions up to 2.1.3 because of the recent fixes to http://java.net/jira/browse/JAVASERVERFACES-1751. However you could still retrieve the value if you issue a request at the right moment (for example by frequently polling for a specific higher sequence number) or if the flash is set to keep the value for more than one request. The fundamental problem lies in the application scoped map.

ren-zhijun-oracle commented 12 years ago

@javaserverfaces Commented @arjantijms said:

Another solution could be to (optionally) store flash data in the HTTP session and thus piggyback on the security of the JSESSIONID.

I just noticed the same thing was proposed as a solution for #1453, but as it appears some users are strongly against ever using the session for flash data:

I just contacted my user advocate at Garmin corporation, who advocated for a "never use the session" option for the flash. I'll try to get a quote from them to sway the EG.

ren-zhijun-oracle commented 12 years ago

@javaserverfaces Commented frederickkaempfer said: A fix would be greatly appreciated, since this security issue currently prevents us from using the Flash scope.

One solution would be to use a GUID instead of a sequential number. Another solution could be to (optionally) store flash data in the HTTP session and thus piggyback on the security of the JSESSIONID.

Instead of replacing the sequence number with GUIDs completely, a GUID could also be added to the cookies value (by adding another separator char (X)) and saved in ELFlash's innerMap (either as part of the key or the value) or an additional Map.

The GUID the clients sends to the server then only needs to be verified by comparing it with the value stored in the map.

EDIT: I also wonder if the cookie could be set to HttpOnly and perhaps Secure (if sent over SSL)?

Another idea for a simple fix: Encrypt and decrypt the flash data similar to the view state using an instance of the ByteArrayGuard class.

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented darious3 said:

Another idea for a simple fix: Encrypt and decrypt the flash data similar to the view state using an instance of the ByteArrayGuard class.

I hope this will get at least a simple fix like the above suggestion soon. For such a serious security issue this has been open for much too long.

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented @manfredriem said: Can you verify if the latest 2.1 release is also affected?

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented frederickkaempfer said: Yes it is.

I have created a reproducer, where should I send it to?

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented @manfredriem said: Please send it to issues@javaserverfaces.java.net. Please also let me know which specific version you tested. Thanks a bunch!

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented frederickkaempfer said: Done. Thanks for looking in to this. As a recap here is what needs to be done

1. Make the numeric values in the csfcfc cookie sufficiently random so they are not guessable (like UUIDs or session ids) 2. Make the cookie HttpOnly. There is no need for it to be modifiable in Javascript and it prevents XSS. 3. Make the cookie Secure if the connection is running over HTTPS.

I have tested with 2.1.20.

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented aschwart said: I sent some comments along the following lines in response to Manfred's email to the Mojarra dev list, but not sure whether everyone here is on that list, so pasting them here as well:

I wanted to suggest that we strongly consider re-parenting the Flash implementation on top of the HttpSession instead of re-inventing session-like functionality on top of Application.

We've discussed this on the expert group more than once, though I could only locate this thread in the archives:

https://java.net/projects/javaserverfaces-spec-public/lists/jsr344-experts/archive/2011-04/message/37

Arjan mentioned this above as well, along with some words of caution:

arjan tijms added a comment - 16/Feb/12 12:35 PM

Another solution could be to (optionally) store flash data in the HTTP session and thus piggyback on the security of the JSESSIONID.

I just noticed the same thing was proposed as a solution for #1453, but as it appears some users are strongly against ever using the session for flash data:

I just contacted my user advocate at Garmin corporation, who advocated for a "never use the session" option for the flash. I'll try to get a quote from them to sway the EG.

I don't remember seeing the follow up from this.

The case for avoiding HttpSession use would have be extremely compelling given that doing so means re-implementing much of the functionality provided by HttpSession at the JSF layer. JSF should be leveraging the underlying platform rather than replacing bits of it.

(Unfortunately I don't have much time to engage in further discussion on this until mid-next week, but wanted to at least share my thoughts before the implementation work gets going.)

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented kithouna said:

The case for avoiding HttpSession use would have be extremely compelling

If the HTTP session is used, the use case for a "You have been logged-out" message on the page where a user is redirected to after a logout would not work anymore.

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented aschwart said: @kithouna - Can you provide more details about your use case? It sounds like you are saying that you required some user-specific state in order to display the logged-out message after invalidating the session. Is that right? What state do you need to display this message?

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented kithouna said: aschwart, it's not that special; in a backing bean I call request.logout and invalidate the session, then I set a faces message, and redirect to the home page. The home page will then say: "You have been logged-out".

This is a very minor, but still important use case. Yes, I could redirect with a ?logout=true request parameter, then have a bean at the index monitor that, and if it's there set the message. But that's just not as nice, and maybe sometimes you want the name of the user in there: "User Peter has been logged-out".

Another use case is much more fundamental; for a web application that's completely stateless and doesn't use sessions at all. Since JSF 2.2 this is now possible and maybe someone already build such a thing before JSF 2.2 and doesn't use forms but only h:links and such. For such apps flash is convenient to store some data that should not be bookmarked between pages.

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented aschwart said: Hi kithouna - sorry about taking so long to follow up. I was offline for a little vacation. Thanks for sharing your use cases.

For the "completely stateless" case, one thing that I am trying to wrap my head around is why storing Flash data at the Application level instead of in the HttpSession is a win. In either case, the app is stateful - ie. the app holds onto the same state across requests regardless of where this state is stored. Does the desire to avoid the HttpSession have to do with the fact that the session itself will stick around for some period after the Flash scope is cleared out? Would using a short session timeout be sufficient to alleviate this? Or is the concern that there is some other overhead that would be present when using the session that Mojarra currently avoids by implementing session-like functionality at the application level?

For the post-logout message/state case, there are some solutions/workarounds available (eg. use a request parameter), but, yeah, these do require doing more work at the app level.

My concern about re-implementing HttpSession-like functionality is that we are missing out on features that are implicitly provided by the app server, eg. Flash scope does not work well with clustering. In the completely stateless case (no HttpSession usage), we are not able to leverage session affinity, and thus might not be able to get back to the correct server (the server which has the current user's Flash data) when running behind a load balancer. Also, it seems like the Flash implementation would need to provide some alternative to the session timeout interval, or else run the risk of leaking Flash state (eg. in the event that no subsequent request ever arrives after storing some state in Flash scope).

My feeling is that Mojarra should provide a Flash implementation that works well with clustering/load balancing out of the box, without requiring any JSF-specific configuration, which I believe means using the HttpSession for storage of Flash data.

I'll leave it up to the Mojarra team to decide whether they think that this is reasonable requirement, and if so whether they want to address it here or via a separate issue.

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented kithouna said: Aschwart, I understand your case. Indeed, it's not that black/white.

You pose a very good question; if we go stateless, why keep some state in app scope? The reason is that one item in app scope is more fine grained. If you create a session it sticks around as long as the client keeps doing requests to the app.

Putting a single item in app scope is a one time thing; it is reaped when the client has consumed the item.

Maybe if we create a session (in case there's none) exactly when the Flash item is created, and kill the session when that Flash item is consumed, we would have roughly the same effect. But since session scope is shared by multiple windows it's more difficult.

Having essentially mini scopes (yes stateful) for every single flash item is more manageable I think, but I''m not the big expert on this topic. I agree that clustering is an issue here!

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented @edburns said: I think it would help to separate out the discussion on this issue into two parts.

1. The original intent of the issues: making the Flash more secure

2. Whether or not flash depends on the session.

Is it safe to separate these? If so, I can create a new issue for part 2.

If I don't hear anything in a few days, that's what I'll do.

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented @edburns said: LU> In my opinion, a random number generator like the one used in Apache LU> Trinidad for its pageFlowScope is enough. The idea here is just make LU> very difficult to guess the next number in the sequence.

I agree.

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented @edburns said: I just committed r12013 to the 2.1.x branch. I want to wait for it to clear and I will then forward port it to 2.2.x branch.

Then I'll go back and migrate the remaining flash tests to the new harness.

Also, I am ready to go to have a hudson job that runs the clustering tests. Manfred, how do you want to work that? Should we tack it onto existing jobs, or have new ones?

Ed

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented @edburns said: in progress. Encrypted value is unnecessary long.

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented @edburns said: checkpoint

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented @edburns said: Commit forward port when <http://hudson-sca.us.oracle.com/view/MOJARRA_ALL/job/MOJARRA_2_1X_ROLLING_DEPLOY/115/> and <http://slc03qna.us.oracle.com:7070/hudson/view/Mojarra%202.1/job/2_1_x-test-gf-3_1_2_2/261/> are clean.

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented @edburns said: Looks like forward port went in clean.

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented frederickkaempfer said: Hello Ed Burns,

great news that this bug is fixed! However one thing that is still missing is Cookie security:

If the request is secure (running over SSL) the Cookie should be set to secure. (Or else cookies from an https-only application will also be sent with every http request).
The cookie should be set to http only (because there is no reason it needs to be available to javascript, thus it prevents some XSS attacks).

This would just be a simple change in PreviousNextFlashInfoManager#encode and would bring the cookie security on par with the jsessionid cookie.

Thanks!

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented File: 20130620-1647-0500-i_moj_2126.patch Attached By: @edburns

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented File: 20130621-2335-0500-i_moj_2126.patch Attached By: @edburns

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented File: diffs.patch Attached By: @edburns

ren-zhijun-oracle commented 13 years ago

@javaserverfaces Commented Issue-Links: is related to JAVASERVERFACES-1449 is related to JAVASERVERFACES-2911 JAVASERVERFACES-2898

ren-zhijun-oracle commented 7 years ago

@javaserverfaces Commented This issue was imported from java.net JIRA JAVASERVERFACES-2126

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented Marked as fixed on Wednesday, June 26th 2013, 9:10:33 am

eclipse-ee4j / mojarra

Flash scope cookie enables data exploits #2130

Affected Versions