From 83fe1d5b7237e29b20552beeb0c72ecbc188dee8 Mon Sep 17 00:00:00 2001
From: "Cathy J. Fitzpatrick" <cathy@cathyjf.com>
Date: Mon, 4 Feb 2013 20:42:38 -0700
Subject: [PATCH] Don't return valid JavaScript from action.php

This avoids an attack where a malicious webpage contains

  <script id="data" type="application/json"
    src="http://play.pokemonshowdown.com/~~showdown/action.php?act=upkeep"/>

The webpage could then read the value of the `data` element using
standard DOM methods in order to steal the user's login assertion
and login as the user on the `showdown` server.
---
 action.php       | 13 ++++++++-----
 js/battledata.js | 10 +++++++++-
 js/sim.js        | 14 +++++++-------
 3 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/action.php b/action.php
index 05898f1ba..711bc6cc2 100644
--- a/action.php
+++ b/action.php
@@ -30,6 +30,7 @@ if (@$_REQUEST['json']) {
 	$multiReqs = true;
 }
 
+$outPrefix = ']'; // JSON output should not be valid JavaScript
 $outArray = array();
 
 foreach ($reqs as $reqData) {
@@ -175,7 +176,7 @@ foreach ($reqs as $reqData) {
 	//		'userserverdata' => @$user['userdata']['psserver'][$servertoken]
 		);
 		header('Content-type: application/json');
-		die(json_encode($serveruserdata));
+		die($outPrefix . json_encode($serveruserdata));
 		break;
 	case 'ladderupdate':
 		include_once 'lib/ntbb-ladder.lib.php';
@@ -200,6 +201,7 @@ foreach ($reqs as $reqData) {
 		$out['p2rating'] = $p2['rating'];
 		unset($out['p1rating']['rpdata']);
 		unset($out['p2rating']['rpdata']);
+		$outPrefix = '';	// No need for prefix since only usable by server.
 		break;
 	case 'prepreplay':
 		include_once 'lib/ntbb-ladder.lib.php';
@@ -224,6 +226,7 @@ foreach ($reqs as $reqData) {
 		} else {
 			$out = !!$db->query("INSERT INTO `ntbb_replays` (`id`,`loghash`,`p1`,`p2`,`format`,`date`) VALUES ('".$db->escape($reqData['id'])."','".$db->escape($reqData['loghash'])."','".$db->escape($reqData['p1'])."','".$db->escape($reqData['p2'])."','".$db->escape($reqData['format'])."',".time().")");
 		}
+		$outPrefix = '';	// No need for prefix since only usable by server.
 		break;
 	case 'uploadreplay':
 		function stripNonAscii($str) { return preg_replace('/[^(\x20-\x7F)]+/','', $str); }
@@ -255,7 +258,7 @@ foreach ($reqs as $reqData) {
 		$user = $users->getUserData($reqData['user']);
 		$ladder->getAllRatings($user);
 		header('Content-type: application/json');
-		die(json_encode($user['ratings']));
+		die($outPrefix . json_encode($user['ratings']));
 		break;
 	case 'ladderformatget':
 		include_once 'lib/ntbb-ladder.lib.php';
@@ -271,7 +274,7 @@ foreach ($reqs as $reqData) {
 		unset($user['rating']['formatid']);
 		unset($user['rating']['rpdata']);
 		header('Content-type: application/json');
-		die(json_encode($user['rating']));
+		die($outPrefix . json_encode($user['rating']));
 		break;
 	case 'ladderformatgetmmr':
 	case 'mmr':
@@ -312,8 +315,8 @@ foreach ($reqs as $reqData) {
 // json output
 if ($multiReqs) {
 	header('Content-type: application/json');
-	die(json_encode($outArray));
+	die($outPrefix . json_encode($outArray));
 } else {
 	header('Content-type: application/json');
-	die(json_encode($out));
+	die($outPrefix . json_encode($out));
 }
diff --git a/js/battledata.js b/js/battledata.js
index e34012c78..1017c1afe 100644
--- a/js/battledata.js
+++ b/js/battledata.js
@@ -297,7 +297,15 @@ var basespecieschart = {
 };
 
 var Tools = {
-	
+
+	safeJson: function(f) {
+		return function(data) {
+			if (data.length < 1) return;
+			if (data[0] == ']') data = data.substr(1);
+			return f.call(this, $.parseJSON(data));
+		};
+	},
+
 	prefs: (function() {
 		var localStorageEntry = 'showdown_prefs';
 		var data = (window.localStorage &&
diff --git a/js/sim.js b/js/sim.js
index 157e520bd..ce561ecf6 100644
--- a/js/sim.js
+++ b/js/sim.js
@@ -1186,7 +1186,7 @@ function Lobby(id, elem) {
 		case 'ladder':
 			if (!target) target = me.userid;
 			var self = this;
-			$.get(actionphp + '?act=ladderget&serverid='+Config.serverid+'&user='+target, function(data) {
+			$.get(actionphp + '?act=ladderget&serverid='+Config.serverid+'&user='+target, Tools.safeJson(function(data) {
 				try {
 					var buffer = '<div class="ladder"><table>';
 					buffer += '<tr><td colspan="7">User: <strong>'+target+'</strong></td></tr>';
@@ -1209,7 +1209,7 @@ function Lobby(id, elem) {
 					self.rawMessage(buffer);
 				} catch(e) {
 				}
-			}, 'json');
+			}), 'text');
 			return false;
 			
 		case 'buttonban':
@@ -2919,7 +2919,7 @@ function overlaySubmit(e, overlayType) {
 			act: 'login',
 			name: name,
 			pass: $('#overlay_password').val()
-		}, function (data) {
+		}, Tools.safeJson(function (data) {
 			if (!data) data = {};
 			var token = data.assertion;
 			if (data.curuser && data.curuser.loggedin) {
@@ -2941,7 +2941,7 @@ function overlaySubmit(e, overlayType) {
 					error: 'Wrong password.'
 				});
 			}
-		}, 'json');
+		}), 'text');
 		overlayClose();
 		break;
 	case 'register':
@@ -2953,7 +2953,7 @@ function overlaySubmit(e, overlayType) {
 			password: $('#overlay_password').val(),
 			cpassword: $('#overlay_cpassword').val(),
 			captcha: captcha
-		}, function (data) {
+		}, Tools.safeJson(function (data) {
 			if (!data) data = {};
 			var token = data.assertion;
 			if (data.curuser && data.curuser.loggedin) {
@@ -2973,7 +2973,7 @@ function overlaySubmit(e, overlayType) {
 					error: data.actionerror
 				});
 			}
-		}, 'json').error(function (e) {
+		}), 'text').error(function (e) {
 			alert('error: ' + e);
 		});
 		overlayClose();
@@ -3232,6 +3232,6 @@ if (!Config.down) {
 		$.post(actionphp, {
 			act: 'upkeep',
 			name: name
-		}, onConnect, 'json');
+		}, Tools.safeJson(onConnect), 'text');
 	}
 }