There may be no code smell ranker than this:

That screenshot was sent by a 3rd-party form vendor who was getting frequent errors from their Marketo API calls and couldn’t figure out what was wrong.
“Does this look right?” they asked. “Nope,” I said immediately — even before they showed me any error messages.
See, even taking Marketo out of the picture, this is wrong for any API. If you see something like this, you’re in the presence of dangerous amateurs.
No way is that value correctly JSON-encoded
It’s clear they’re not encoding since there are double quotes outside the template variable. A JSON-encoded string always includes the double quotes. (Simple JavaScript example: JSON.stringify("Hello")
is "Hello"
with the quotes, not Hello
.)
Let’s review how their “bizarro JSON” logic works.
They’re using Jinja templates, but any template language works the same way. Since there’s no filter specified, the property form.comments
is output exactly as stored on the back end with no encoding, replacing the token {{ form.comments }}
.
Assume the raw value is:
I like your style.
Since they’re not encoding at all, they end up with this JSON:
{
"comments" : "I like your style."
}
So far, so (misleadingly) good.
But assume the raw value is:
Please send a sample of your industrial tiles.
I’m in Denmark, hope it won’t take too long.
You end up with this totally broken payload (remember, JSON doesn’t allow literal line breaks inside strings):
{
"comments" : "Please send a sample of your industrial tiles.
I’m in Denmark, hope it won’t take too long."
}
Now you start to see the problem. Any character that requires encoding breaks the whole thing. But it gets worse...
... way worse.
The above examples presume innocence. But user-supplied input is never safe to output as-is. Doesn’t matter how much validation you think you’re doing: databases will let you store arbitrary data in a string field, and crazy stuff will get in there.[1]
Imagine an attacker purposely posts this value in the comments field on your custom form:
Hi", "secretKey" : "abc123
The back end injects that value into the template as-is, so the assembled API payload becomes:
{
"comments" : "Hi", "secretKey" : "abc123"
}
The attacker just tricked you into updating the field secretKey
with a value of their choosing, while you think you’re just updating comments
. That’s a crazy injection attack vulnerability! It’s not as familiar as SQL injection or command injection, but should be just as horrifying.[2]
And THAT’s why you always JSON-encode
Any template language worth using has a JSON filter or will let you add a custom one. In Jinja, it’s tojson
. So the correct template is:
{
"comments" : {{ lead.comments | tojson }}
}
Now all payloads above are correctly escaped and neutralized:
{
"comments" : "I like your style."
}
{
"comments" : "Please send a sample of your industrial tiles.\n\nI’m in Denmark, hope it won’t take too long."
}
{
"comments" : "Hi\", \"secretKey\" : \"abc123"
}
All of those only update the comments
field, as intended.
If you’re not explicitly using JSON encoding — something that has the 4 letters j
-s
-o
-n
in it — it’s 99% likely your code has the same vulnerability.
Notes
[1] There may be special-purpose databases out there that do permanent validation within the storage engine (meaning not just BEFORE INSERT/UPDATE
triggers, not just CHECK
constraints, but some compiled-in feature that can’t be turned off). Never seen such a thing myself, and I’ll bet your company isn’t using one!
[2] Such vulnerabilities don’t just let attackers overwrite values, they may allow reading existing values too. What if the API lets you include a list of fields to return, which you think you’ve hard-coded in the template:
{
"getFields": [
"firstName",
"lastName"
],
"setFields": {
"comments" : "{{ form.comments }}"
}
}
Send this carefully crafted hack as Comments:
Hi" }, "getFields": [ "password" ], "closer" : { "a" : "b
Now the JSON payload is:
{
"getFields": [
"firstName",
"lastName"
],
"setFields": {
"comments" : "Hi" }, "getFields": [ "password" ], "closer" : { "a" : "b"
}
}
Oops! You’ve successfully made the API response include the password
.
And if the back end relays the response to the browser — reasonable if you expect it to only contain “safe” fields — you’ve got a major compromise.