10

I understand that the use of eval(json_str) on the client is vulnerable to malicious code. My question is, if json_str was an array constructed by the PHP function json_encode, would I be safe?

For example,

json_str = json_encode(array(record1, 
                             record2, 
                             record3));

would it now be entirely safe to use eval(json_str) inside client-side code?

HamZa
  • 14,671
  • 11
  • 54
  • 75
whamsicore
  • 8,320
  • 9
  • 40
  • 50
  • 5
    Don't use eval. It is slow and hard to debug. Use a [real parser](https://github.com/douglascrockford/JSON-js) – Quentin Jul 25 '11 at 13:26

5 Answers5

23

In terms of pure JavaScript, yes, you are safe: the output of json_encode can never containing anything but static values which will have no unexpected side effected when passed to eval. (Though you typically have to surround your JSON string with () when using eval, to avoid it misinterpreting an object literal expression as a statement block.)

Aside: this is not necessarily true of all JSON encoders because there are some characters that are valid to include raw in a JSON string that are not valid raw in JavaScript. Most notably, U+2028 and U+2029 which can't go unescaped in JavaScript string literals as they constitute newlines. However PHP's encoder encodes all non-ASCII characters (eg as "\u2028") so no issue here.

In terms of JavaScript embedded in another language (typically: HTML) you are not necessarily safe. For example:

<script type="text/javascript">
    var v= <?php echo json_encode($value); ?>;
</script>

In this example, what if value contains a string with the character sequence </script? This would allow the value to end the script block prematurely and thus escape into HTML markup, where it could then inject other malicious script.

To avoid this problem, when including JSON content in HTML, always encode the < character in string literals, as \x3C or, in JSON-compliant terms, \u003C. For compatibility with XHTML non-CDATA script blocks, do & as well. For compatibility with JS inside event handler attributes, do quotes as well.

PHP will do this for you with the right options to json_encode():

var v= <?php echo json_encode($value, JSON_HEX_QUOT|JSON_HEX_TAG|JSON_HEX_AMP|JSON_HEX_APOS); ?>;

(You may want to define a shortcut function to make this quicker to write.)

bobince
  • 528,062
  • 107
  • 651
  • 834
  • What happens if `json_encode` is maliciously overwritten by some other function? What happens if some of the scripts are subverted to deliver a virus? There's no reason to let the *entire* application crash and burn just because *part* of it was compromised. – zzzzBov Jul 25 '11 at 14:02
  • 4
    @zzzzBov: If one of your functions has been overwritten, you have already completely lost. In JavaScript and PHP there is insufficient encapsulation to protect any part of an application from any other part of it (unless you go multi-document using different domains), so there is absolutely nothing further to lose by calling a function. If you can't call a function out of fear it has been sabotaged, you're not going to get much (read: anything) done. – bobince Jul 25 '11 at 16:23
  • I'm not saying dont use `json_encode`; it is *the* way to encode json in PHP. What I'm saying is that there's no reason to assume that it is infallible. If tomorrow it was announced that there was an attack vector that enabled the attacker to inject some additional JS into the output of `json_encode`, I wouldn't have to rewrite any of my JavaScripts. I *might* have to rewrite my PHP. If you take on "the war's already lost" mentality, then you're going to be writing inherently bad code. – zzzzBov Jul 25 '11 at 17:30
  • 14
    `json_encode` with default flags escapes slashes so it would give `"<\/script>"`, so this is fine as `<\/script>` doesn't terminate the script tag in html. You would have to explicitly use `JSON_UNESCAPED_SLASHES` flag to get bitten by this. – Esailija Aug 01 '12 at 22:07
  • 1
    @Esailija: Yep, this is a new feature (and a welcome default) in PHP 5.4. We probably still need `JSON_HEX_QUOT|JSON_HEX_TAG|JSON_HEX_AMP|JSON_HEX_APOS` until that version is more widespread, or for script-in-XHTML compatibility. – bobince Aug 02 '12 at 07:32
  • 10
    @bobince, this isn't new in PHP 5.4. echo json_encode('') outputs <\/script> in PHP 5.2.9 – David Barnes Jan 29 '13 at 19:02
  • @bobince I always wrap `JSON.parse` around the stuff on the right so the client sees `var v = JSON.parse(...output of json_encode...)` - are you saying this is a waste of time? – Flash Apr 15 '13 at 07:08
  • @Andrew: the input to `JSON.parse` has to be a string, so if you wanted to do this you'd have to encode the whole JSON output as a JavaScript string literal. As that's usually done with `json_encode`, you'd have to double-encode, which does seem a bit waste-of-timey... – bobince Apr 15 '13 at 08:49
  • @bobince With python I do this: `'var v = JSON.parse("%s")' % escapejs(json.dumps(value))` (with Django's [escapejs](https://docs.djangoproject.com/en/dev/ref/templates/builtins/#escapejs) string escaper) so the browser is JSON.parsing a safe string. My spider senses tell me it's dangerous to write `var v = [code]` and hope the code creates a valid object - it's sort of abusing the fact that JSON notation happens to also be valid object syntax... don't you think? – Flash Apr 15 '13 at 10:38
  • @Andrew: JSON was specifically designed as a subset of object syntax, so I don't think it's really abuse - as mentioned above though, the characters U+2028 and U+2029 are where that design failed. Python `json.dumps` will escape all non-ASCII by default, so that's not a problem unless you specifically disable `ensure_ascii`. The other point discussed is embedding that value in HTML. `json.dumps` doesn't JS-encode HTML metacharacters, so you would still have to take care of that separately (simple string replace is OK since `<&"` can never appear in JSON outside of string literals. – bobince Apr 15 '13 at 12:04
  • Django's `|escapejs` is not supposed to take care of HTML embedding problems - the doc specifically says "This does not make the string safe for use in HTML", so in principle you still have a problem with your example code if it's inside a ` – bobince Apr 15 '13 at 12:06
  • Personally for what it's worth - my preferred approach is to go with `JSON.parse`, but using a string extracted from an ``. Injecting the data into HTML instead of JavaScript means I can use a standard HTML escape instead of having to worry about JS-escaping - and eventually it gives you the potential to remove all inline JavaScript from your HTML, which has advantages like being able to deploy tight Content Security Policy settings. – bobince Apr 15 '13 at 12:09
  • That's a nice solution, thanks. The problem with the `escapejs` approach too is that it ends up _really_ long which is annoying and a waste. And like you say, you end up with a ` – Flash Apr 15 '13 at 12:24
  • **WARNING:** Escaping quotes is not enough for it to be safe to use inside event handlers. See [this](https://security.stackexchange.com/a/175633/98538). – Anders Dec 17 '17 at 00:28
  • @bobince,if I use `JSON_HEX_QUOT|JSON_HEX_TAG|JSON_HEX_AMP|JSON_HEX_APOS` in php `json_encode`,can I correctly back to origin when php `json_decode`? – kittygirl Mar 20 '19 at 08:26
4

If you want to use Content Security Policy (CSP), you are prevented from executing inline script tags. This would therefore would make bobince's otherwise amazing answer impossible since CSP requires that all JavaScript be in separate files.

How to access dynamic JSON in external JavaScript:

One way around this is to html encode the JSON with PHP (which should prevent XSS since json_encode converts / to \/) and then echo it to a data block (a script tag with a non-Javascript MIME type) and then use JavaScript to get the contents of that tag (adapted from OWASP):

Put this inline. Note, even if not using CSP, it won't actually be executed by the browser due to the type attribute:

<script id="jsonString" type="application/json">
<?php
echo json_encode($object, JSON_HEX_QUOT|JSON_HEX_TAG|JSON_HEX_AMP|JSON_HEX_APOS); 
?>
</script>

...or use a display: hidden HTML element instead:

<div class="display-hidden">
<?php
echo json_encode($object, JSON_HEX_QUOT|JSON_HEX_TAG|JSON_HEX_AMP|JSON_HEX_APOS); 
?>
</div>

Then in your external JavaScript file:

var dataElement = document.getElementById('jsonString');
var jsonString = dataElement.textContent || dataElement.innerText;
var jsonObj = JSON.parse(jsonString);
Mike
  • 23,542
  • 14
  • 76
  • 87
  • I referred to this answer on [security.SE](https://security.stackexchange.com/q/261347/176611), questioning if `htmlspecialchars` is necessary inside a data block. – August Janse Aug 08 '22 at 12:11
  • @AugustJanse Good point. I've refactored my answer. – Mike Aug 08 '22 at 17:57
1

Don't use eval for JSON parsing

Don't do it.

It's very likely that your server will never become compromised, and that your application will be mostly secure, blah blah blah, that's not the point. It is possible for a server to become partially compromised (too many vectors of attack, what if the php json_encode function became compromised on your server?).

The simple solution is not to trust anything sent by anyone. Modern browsers have native JSON parsers, and www.json.org provides a long list of JSON parsers for various different languages. The JS versions will fall back on the native implementation for speed.

What all this means is that there's no good reason to use eval for JSON parsing.

Community
  • 1
  • 1
zzzzBov
  • 174,988
  • 54
  • 320
  • 367
  • So how do send my array from PHP to JS exactly? – whamsicore Jul 26 '11 at 18:55
  • @whamsicore, you send JSON from php using `json_encode`, but you ***do not*** use `eval` to parse the data; you use `JSON.parse(data)`. The difference is that if `json_encode` became compromised and sent `"(function(){...malicious code here...})();"`, you'd be automatically calling that malicious code using `eval`. `JSON.parse` on the other hand will throw an error which will prevent your site from causing harm to your users. – zzzzBov Jul 26 '11 at 19:56
0

It should be safe, but on the client, you aren't guaranteed that the json_str hasn't been injected by some other source.

Daniel A. White
  • 187,200
  • 47
  • 362
  • 445
  • True, but it's sort of outside the scope of the question. The php_encode()d string *is* safe, whether whamsicore has adequately secured the rest of their app to prevent someone injecting code and fooling the eval()ing part is something else. – El Yobo Jul 25 '11 at 13:30
0

Yes and no:

Yes: PHP produces valid JSON

No: PHP may as well return malicious code as in JSON.

If you can trust the source, or if you even have full control over it (because its yours), there is no problem.

KingCrunch
  • 128,817
  • 21
  • 151
  • 173
  • 2
    Don't ever trust the source. As silly as it sounds, it's possible that the server was only partially compromised, and that the data being served up is malicious. Use a JSON parsing lib, they'll default to native JSON parsing if the browser already has it. – zzzzBov Jul 25 '11 at 13:55