Bug Referer filter can be circumvented

[matchu]

Quick preface: I'm assuming that Mafia validates Referer headers to ensure that evil.com can't send an evil request to the KoLMafia relay browser. Is that right? If there's some other reason, well, disregard the rest of this post, kthx

If that's the intent of the security feature, then here's a technique to bypass it:

1. evil.com creates an anonymous iframe that points to a page with no real URL (e.g., with a "javascript:'HTML goes here'" src)
2. The anonymous iframe then submits a request to "http://127.0.0.1:60080/do-something-evil"
3. Mafia receives the request, which has no Referer header because the anonymous iframe has no URL.
4. Mafia therefore misidentifies the request as valid and it is processed, even though it's actually evil and therefore should be rejected.

I'm currently, uhh, abusing this very issue in my AoJ planner to show AoJ skill descriptions, so that can serve as proof-of-concept. (Toggle the "KoLMafia relay browser?" checkbox, login via Mafia, and then hover over a skill icon.) I'll gladly do my own mirroring if y'all choose to patch this issue, but this is the quick fix we're using in the meantime.

Therefore, since an evil site can strip a Referer header, Mafia should assume that all requests with no Referer header are evil and therefore block them. (I can understand a compatibility argument, since not all clients send Referer headers at all, but those clients are also uncommon and/or ancient. Security wins here.)

Then again, I'm not entirely sure what the Referer check gains us, because KoL itself already has plenty of CSRF protection. Is this to defend against relay scripts that don't bother with these kinds of protections? I don't know enough about the relay browser infrastructure to comment on that :/

Anyway, hope this helps Love all the hard work y'all do

 

[lostcalpolydude]

I wasn't seeing this issue in Firefox (and I was about to post that the feature in your tool doesn't even work), but then I opened up other browsers and saw it work in IE and Chrome.

It looks simple to block requests with no referer, I don't know if that would break anything in mafia.

Edit: It took me about 2 minutes to determine that just opening the relay browser has a null referer, so this isn't so trivial to fix.

 

[Theraze]

Regarding 11126, if you turn off security yourself, that's on you. But base-mafia should be reasonably secure and not allow for malicious redirectives.

 

[lostcalpolydude]

Regarding 11126, if you turn off security yourself, that's on you. But base-mafia should be reasonably secure and not allow for malicious redirectives.

You probably saw an earlier version of my post where I mentioned that revision, but that didn't actually make anything worse probably. There used to not be a check of the referer unless the request included one, now the check explicitly allows a null referer instead, for effectively the same result (except an empty string for the referer is now allowed, when it wasn't before, I think).

 

[matchu]

Oh, ha, duhh. Yeah, I guess that's the one big, glaring case where no-Referer is important xD One fix would be to only allow no-Referer requests to game.php, but this is another case where I don't know enough about the relay browser to know if that's sufficient. Are there other Mafia features that open a relay browser window to paths other than game.php?

Also, huh. I thought I'd tested in Firefox and it had worked, but I guess I tested the local version that works anyway.

 

[lostcalpolydude]

It looks like allowing game.php and favicon.ico when there is no referer is sufficient. If anyone else wants to test and see if other things are broken by the change, I locally changed webui/RelayAgent lines 342-345 to

		if ( referer == null || referer.equals( "" ) )
		{
			return this.path.equals( "/game.php" ) || this.path.equals( "/favicon.ico" );
		}

 

[lostcalpolydude]

I see issues with charpane.php when I enter a choice adventure too.

Also, this doesn't seem like the right way to fix the problem, since a malicious page with a null referer could always be game.php (and probably would be, if the attacker knew what they were doing).

 

[matchu]

Can a request to game.php be evil? Due to browsers' strict cross-domain security policies, I could create a frame to game.php, but I wouldn't be allowed to control that frame except to change its src all over again, in which case the original request to game.php was redundant. Are there potentially dangerous query parameters on game.php? If not, there shouldn't be an issue with that. CSRF attacks are write-only: I can trick the client into sending a request, but I can't read the results or in any way interact with the page. So, the only danger is when I can craft a request to do something, and the server accepts it. Requests that don't change state on the client or server are literally always okay.

The attack vector would be something more like pointing a frame to… well, I'm not exactly sure. Whatever action the feature is trying to protect. Relay script interfaces? All the legit actions I can find in KoL and in the relay browser already seem to be CSRF-protected with that pwd parameter or whatever.

…okay, here's another option: use standard CSRF protection everywhere (it looks like we already do), and only bother with referer protection for places where we can't trust scripts to have implemented it properly, e.g., .ash URLs. Those definitely won't receive null referers naturally, so we can block 'em there without issue.

 

[lostcalpolydude]

An attack would probably have a hard time benefiting the attacker, but there are things that can hurt those affected. Or maybe just one thing (adventuring), since I can't think of anything else now. Maybe require a non-null referer if the path has ? in it (checking a URL-decoded version too, possibly)?

 

[matt.chugg]

An attack would probably have a hard time benefiting the attacker, but there are things that can hurt those affected. Or maybe just one thing (adventuring), since I can't think of anything else now. Maybe require a non-null referer if the path has ? in it (checking a URL-decoded version too, possibly)?

IDEA:

Have mafia create a unique session ID when the relay browser is launched and pass it in a query string to the launch url, (eg game.php?session=someGUID)

The first request mafia will then see will be to game.php and will also contain the query string which can obviously be removed before passing on to KoL,

In the response to this first request, (once relayed from KoL) push the same value it into the set-cookie header before passing on to the browser, now, the browser has a cookie mafia can check for in every request against the internal id from when the browser was launched, pretty sure an iFrame wouldn't inherit this cookie as that would be a serious xss issue.

I haven't looked at how the requests to KoL are generated, but I presume headers aren't all directly passed straight on so shouldn't interfere with KoL requests

Just an idea, not sure how easily implemented this is, or if i'm way off base anyway

EDIT: doesn't even need to be game.php, mafia will see all requests to its internal server, send the browser to anything to create the session then send a 302 to game.php

 

[matchu]

Hmm, good point on adventures—weird that so many things are CSRF-protected, but that one isn't. Then again, if KoL doesn't protect against it, it might not be our job to protect against it, anyway. It'd definitely be good, but, if we can't find a satisfactory solution that does so, it's not all that bad :/

Pages from good.com embedded on evil.com usually do use the cookies from good.com. This is what folks talk about when they say "third-party cookies", and it's enabled by default in most browsers. (Browsers protect against security issues by not letting evil.com interact with the contents of the frame.) Additionally, I can still do an insta-redirect from evil.com to good.com, which will definitely keep the cookies, regardless of browser settings.

I think it's reasonable to say that requests without query parameters (GET or POST) are safe with null referers. It wouldn't generalize well to the internet as a whole, but, given how the KoL team tends to do things that way rather than building parameters into pathname or anything like that, it's a decent assumption here.

 

[lostcalpolydude]

12023. I did a few things with this. First, all requests are allowed regardless of referer if they have a valid password hash (once I saw that the "wait" command generated by an extended choice adventure somehow has a null referer, I decided this was the most reasonable approach).

Second, a null/empty referer is allowed for any request without "?" in it. Perhaps that should do some URL decoding to be a better check, but even without that it's a more strict check than before.

Third, I decided to allow all desc_ URLs regardless of referer. It doesn't hurt anything, I like being able to pop open a new window to view those sometimes (for new content that mafia can't auto-parse, for instance), and allowing tools like the one that prompted this report seems fine. Also, some people on the wiki suggested adding popup links to open in mafia like the ones that work for vanilla KoL, and I think a lot of people would find it convenient if that ever happens.

 

[matchu]

Sweet deal. Thanks for all your hard work, bro

 

[roippi]

Good stuff.

 

[matchu]

An interesting security note: the path "desc_skill.php/../main.php?foo=bar" is improperly accepted by KoLMafia and resolved by the server to "main.php?foo=bar". However, I'm pretty sure that all modern browsers resolve URL paths before submitting them (I had to send the request via cURL rather than the browser, since the browser rewrites it as "main.php?foo=bar" anyway), so I'm not sure that's really an issue. I've confirmed that current Chrome and Firefox resolve the path, and I'd guess that IE and Safari do, too, but it'd be worth checking.

Strictly speaking, it might be wise to have KoLMafia fully resolve the path, too… but if modern browsers aren't vulnerable to this attack, anyway, it's probably unnecessary complexity. Food for thought. Also worth considering if there are any other clever ways to start a path with "/desc_" and then turn it into something else…

 

[Yvain]

The new code may be a bit too strict. Mafia managed to block itself for me when I tried People -> Read Kolmail from the menu.

Request from bogus referer ignored
Path: "/game.php?mainpane=messages"
Host: "127.0.0.1:60080"
Referer: "null"

I'm using r12024.

 

[Theraze]

When I click on the "script" button at the top of fights to automate combat in the relay browse, I get this:

Request from bogus referer ignored
Path: "/fight.php?action=script"
Host: "127.0.0.1:60080"
Referer: "null"

Also using r12024.

 

[lostcalpolydude]

For the first one, I think I can safely whitelist anything starting with game.php to handle any browser windows mafia might want to open from menus.

The second is because the page is set to refresh every second, and the fight takes more than 1 second to run. I'm not sure what to do about that, I could add the password hash I guess.

 

[Theraze]

It actually happens when the second generated macro gets sent. The first macro execution and page-load works fine... the second runs but the page load fails and gives the blank page/connection reset that fronobulax reported in the community thread.

 

[Xande1]

Just for the record, if you have network.http.sendRefererHeader in Firefox set to 0, logging in to KoL directly and playing works fine, but playing through the relay browser becomes completely broken after this change. Just pointing it out because I'm not sure why one works and the other doesn't.