Subject: secdir review of draft-ietf-jsonbis-rfc7159bis-03

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 7 Mar 2017 19:48:24 -0600

Hi all,

I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the
IESG.  These comments were written primarily for the benefit of the
security area directors.  Document editors and WG chairs should
treat these comments just like any other last call comments.

It is rather surprising to find notable omissions with what is
effectively rfc4627ter, though that seems to be my conclusion after
performing this review.  Luckily, they should be easy to resolve
with just a touch more text in the security considerations, for the
most part.

As an overall summary, this document is a simple, clean, crisp
writeup of the JSON interchange format, which is expected to match
that of the ECMAscript standard.

Since this is a data format and not a protocol, any security issues
with this document are expected to be second-order effects, relating
to how the format is used or could be (mis)used by consuming
protocols.  It is useful to give guidance on issues that may occur
due to known-bad implementations, edge cases that can cause trouble,
etc., and this document does so, noting that the fields in an object
are unordered (but some implementations respect the order in their
APIs), when comparing field names the potential for escaped
characters must be taken into account, many implementations have
limited precision/bounds for numbers, non-number mathematical values
cannot be expressed in JSON, etc.

However, I think that the security considerations section would
benefit from some discussion of potential/example consequences of
failing to heed those issues:

Doing name comparison without respect for excaping could let an
attacker inject unsanitized data into what is supposed to be a
trusted structure, potentially giving privilege escalation,
compromising authentication and authorization, etc.

Considering only the first (or last) of duplicated name/value pairs
can lead to vulnerabilities by bypassing security checks, in mixed
environments.  Similarly, a reliance on fields being returned in
order could cause issues (certainly denial of service, potentially
worse) if an attacker uses a different order than expected.  Perhaps
the start of section 4 could refer forward to discussion in the
security considerations, which give some exposition on the
consequences of breaking the "names within an object SHOULD be
unique" guidance.

Implementations should probably check that their numerical routines
do not attempt to emit "NaN" or "Inf" or simlar, though I do not
have a good picture of how that would affect real systems
(hopefully, just a parse error on the receiver, as those are not
permitted by the grammar).  Similarly, the use of extremely large or
precise numbers in mixed environments can lead to unexpected
results.  I expect that there could be vulnerabilities here, such as
if code can be induced into a loop with incearingly large values,
where a sender ends up producing numbers larger than can be
represented on the receiver, which then saturates its
strtonum-equivalent and hits an unexpected codepath.

I'm also concerned about the freewheeling use of Unicode.  While
this document does discuss the potential encodings and lists UTF-8
as the default (and most interoperable), I think it would benefit
from a stricter warning that parties using JSON for communication
must have some out-of-band way to agree on what encoding is to be
used.  I would expect that this is usually going to be done by the
protocol using JSON, but could see a place for the actual
communicating peers to have out-of-band knowledge.  (An application
having to guess what encoding is being used based on heuristics is a
recipe for disaster.)

Additionally, the document makes no mention of Unicode
normalization, which can be a minefield.  The precis working group
has a lot of work in this area, from which the executive summary is:
it's a lot of work to do things correctly, and being sloppy usually
leads to vulnerabilities.  The most obvious issue would be in (the
comparison of) field names using strings that can be represented
differently in different normalization forms (for example, e with
acute accent), which can be either U+00e9 or U+0064 and the
combining character U+0301.  Simply converting to Unicode code
points is insufficient for an implementation to cause those strings
to compare as equivalent.  I think this document should at least
mention that Unicode normalization forms exist and should be
considered by protocol designers when using JSON with characters
outside of US-ASCII.

Section 9 (parsers) mentions that an implementation "may set limits"
on various parameters.  That seems like a good place to give some
guidance on what limits are in common use and how protocols or
protocol peers might discover and/or negotiate each others'
implementation limits.

With the proposed additions to the security considerations out of
the say, on to some other minutiae.

Section 3 describes that "[a] JSON value MUST be an object, array,
number, or string, or one of the following three literal names:
false null true"; in the introduction, this (equivalent) list is
given as "object, array, number, string, boolean, or null", in the
guise of "four primitive types (strings, numbers, booleans, and
null) and two structured types (objects and arrays)".  There may be
some value in using consistent terminology between the two
locations, but it would be pretty minor value and might not be worth
the effort.

The format for numbers (Section 6) permits an explicit (redundant)
plus sign in the exponent part, but not in front of the overall
number.  This feels slightly strange to me, but I don't know if
there are any implementations that get this wrong.  (I also don't
really see security consequences of getting it wrong.)  Similarly,
the fractional part must contain a digit (that is, a number ending
in a decimal point is forbidden), which is probably harmless.  I'll also
note that the number 3.141592653589793238462643383279 is used as an
example of a precision unrepresentable in 64-bit IEEE 754
floating-point, but the next digit in the expansion of pi is a '5',
which would round up to ...80.  Maybe another digit should be added
to keep overly pedantic readers from worrying about rounding modes :)

Should section 8.3 (string comparison) use any RFC 2119 language
with respect to treating strings as equal that use escapes (or even
normalization forms, as covered above)?

The IANA considerations (section 11) might be a little more explicit
that the IANA "Media Types" registry is what is being modified.
I'm also rather curious about the claim that no "charset" parameter
is needed as it "really has no effect on compliant recipients".  Why
is this not a good way to communicate whether UTF-8, UTF-16, or
UTF-32 is in use for a given text?

It might also be useful to have an example of an array object that
contains elements of different type, to reinforce that this is
permitted.

With respect to section 1.3, I personally am more used to seeing a
"changes since <previous document>" section that lays out what
changed inline, as opposed to with (opaque) references to specific
errata.  But that is rather an editorial issue so my personal
preference should not bind you.

-Ben

---

From: Barry Leiba <[email-masked]>
Date: Tue, 7 Mar 2017 21:52:37 -0500

Hi, Ben.

> considered by protocol designers when using JSON with characters

I believe that all of this is the realm of the protocol *using* JSON,
and doesn't belong in the JSON spec itself.  The JSON spec makes it
clear what the encoding options are, and leaves things such as the set
of allowed characters (and any restrictions on them), the
normalization and canonicalization, and the comparison rules to the
next level... and I believe that's how it should be.  Different uses
of JSON will have different needs in these regards, and *those*
specifications are the right places to say that.

Barry

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 7 Mar 2017 21:30:33 -0600

Hi Barry,

> specifications are the right places to say that.

I agree that it is appopriate for the JSON spec to merely list out the
options and leave decisions to the consuming applications/protocols.
However, it seems irresponsible to not mention that those designing
such protocols should be aware of the potential issues.

-Ben

---

From: Julian Reschke <[email-masked]>
Date: Wed, 8 Mar 2017 08:39:50 +0100

On 2017-03-08 02:48, Benjamin Kaduk wrote:
> ...

AFAIU, there is no need for out-of-band knowledge (which would be very 
bad). Recipients are supposed to inspect the payload and detect which of 
the three encoding was used.

That said, we probably should make that clearer.

 > ...
> ...

It might have been, but that's now how it is implemented.

Best regards, Julian

---

From: Julian Reschke <[email-masked]>
Date: Wed, 8 Mar 2017 08:47:24 +0100

> It might have been, but that's now how it is implemented.

s/now/not/

---

From: Carsten Bormann <[email-masked]>
Date: Wed, 8 Mar 2017 09:49:46 +0100

> It might have been, but that's now how it is implemented.

Indeed, and the reality is that JSON over UTF-16 or UTF-32 simply does not exist as an interchange format, so the algorithm is to always just assume UTF-8.

Grüße, Carsten

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 8 Mar 2017 23:53:48 -0600

> > That said, we probably should make that clearer.

If that's what's supposed to happen, it should probably be more
clear, yes.  (But aren't there texts that have valid interpretations
in multiple encodings?)

> > It might have been, but that's now how it is implemented.

Alas.

Thanks for the insight.

-Ben

---

From: John Cowan <[email-masked]>
Date: Fri, 10 Mar 2017 21:08:35 -0500

On Thu, Mar 9, 2017 at 12:53 AM, Benjamin Kaduk <[email-masked]> wrote:

> clear, yes.  (But aren't there texts that have valid interpretations

Not if the content is well-formed JSON and the only possible encodings are
UTF-8, UTF-16, and UTF-32.  It suffices to examine the first four bytes of
the input.  If there are no NUL bytes in the first four bytes, it is UTF-8;
if there are two NUL bytes, it is UTF-16; if there are three NUL bytes, it
is UTF-32.  This works because the grammar requires the first character to
be in the ASCII repertoire, and the NUL *character* (U+0000) is not allowed
at all.

-- 
John Cowan          http://vrici.lojban.org/~cowan        [email-masked]
I don't know half of you half as well as I should like, and I like less
than half of you half as well as you deserve.  --Bilbo

---

From: Julian Reschke <[email-masked]>
Date: Sat, 11 Mar 2017 07:12:45 +0100

On 2017-03-11 03:08, John Cowan wrote:
>
> On Thu, Mar 9, 2017 at 12:53 AM, Benjamin Kaduk <[email-masked]
> <mailto:[email-masked]>> wrote:
>
> Not if the content is well-formed JSON and the only possible encodings
> are UTF-8, UTF-16, and UTF-32.  It suffices to examine the first four
> bytes of the input.  If there are no NUL bytes in the first four bytes,
> it is UTF-8; if there are two NUL bytes, it is UTF-16; if there are
> three NUL bytes, it is UTF-32.  This works because the grammar requires
> the first character to be in the ASCII repertoire, and the NUL
> *character* (U+0000) is not allowed at all.

Good explanation. Maybe the spec should include it.

Best regards, Julian

---

From: [email-masked]
Date: Sat, 11 Mar 2017 07:41:17 -0800 (PST)

> Good explanation. Maybe the spec should include it.

+1

This exact issue just came up in a media type review, where someone
specified a charset parameter because they weren't aware of this algorithm.

It would be very helpful to have this text in the RFC.

				Ned

---

From: John Cowan <[email-masked]>
Date: Sat, 11 Mar 2017 21:58:40 -0500

On Sat, Mar 11, 2017 at 10:41 AM, Ned Freed <[email-masked]> wrote:

> It would be very helpful to have this text in the RFC.

Feel free, whoever is the editor now, to add it.

-- 
John Cowan          http://vrici.lojban.org/~cowan        [email-masked]
But that, he realized, was a foolish thought; as no one knew better than
he that the Wall had no other side.
        --Arthur C. Clarke, "The Wall of Darkness"

---

From: Peter Cordell <[email-masked]>
Date: Sun, 12 Mar 2017 09:06:24 +0000

> It would be very helpful to have this text in the RFC.

Although it does need slightly more detail to take into account 
endian-ness in the case of UTF-16 and -32.

The XML spec may offer some example text:

https://www.w3.org/TR/2008/REC-xml-20081126/#sec-guessing

Pete Cordell
Codalogic Ltd
Read & write XML in C++, http://www.xml2cpp.com

---

From: Carsten Bormann <[email-masked]>
Date: Sun, 12 Mar 2017 10:09:46 +0100

> It would be very helpful to have this text in the RFC.

That would be a regression.

A better change would be to remove the fiction that JSON exists in UTF-16 or UTF-32 forms.

Grüße, Carsten

---

From: Julian Reschke <[email-masked]>
Date: Sun, 12 Mar 2017 10:14:04 +0100

> endian-ness in the case of UTF-16 and -32.

Does anybody recall why we removed 
<https://tools.ietf.org/html/rfc4627#section-3>:

> 3.  Encoding
>
>    JSON text SHALL be encoded in Unicode.  The default encoding is
>    UTF-8.
>
>    Since the first two characters of a JSON text will always be ASCII
>    characters [RFC0020], it is possible to determine whether an octet
>    stream is UTF-8, UTF-16 (BE or LE), or UTF-32 (BE or LE) by looking
>    at the pattern of nulls in the first four octets.
>
>            00 00 00 xx  UTF-32BE
>            00 xx 00 xx  UTF-16BE
>            xx 00 00 00  UTF-32LE
>            xx 00 xx 00  UTF-16LE
>            xx xx xx xx  UTF-8

?

Best regards, Julian

---

From: [email-masked]
Date: Sun, 12 Mar 2017 04:56:29 -0700 (PDT)

> That would be a regression.

On the contrary - what you have now is incomplete if not actually broken, and
this completes/fixes it.

You have a specification that says (a) JSON can be encoded in UTF-8, UTF-16, or
UTF-32 and that (b) That no charset parameter is needed to distinguish the
charset and moreover one has "no effect". But you don't explain how why this is
the the case, or what compliant implementation do to determine the charset.

If you believe clarifying this is problemtic in some way, you need to explain
why.

> A better change would be to remove the fiction that JSON exists in UTF-16 or

That's of course another way to fix the problem. But it's a far more
significant change, and I have to wonder if there is a consensus to do it.

Personally, I have no preference as to the approach that's used. But moving
forward as-is is really not acceptable.

				Ned

---

From: Carsten Bormann <[email-masked]>
Date: Sun, 12 Mar 2017 16:31:42 +0100

On 12 Mar 2017, at 10:14, Julian Reschke <[email-masked]> wrote:
> 
> Does anybody recall why we removed <https://tools.ietf.org/html/rfc4627#section-3>:

I seem to remember that the advice simply is no longer working since JSON was extended from 4627 to 7159.  Instead of trying to come up with an updated algorithm, the WG recognized that this is not a real-world problem.

If 7159bis is supposed to be an Internet Standard, it should focus on the variants that actually interoperate.  Re-encoding JSON into UTF-16 (or UTF-32) is not really interoperable with today’s implementations*).

Grüße, Carsten

*) (Given the large number of JSON implementations, I don’t doubt you will find two implementations that do interoperate on this.  It is not the widely interoperable “JSON” thing, though.)

---

From: Julian Reschke <[email-masked]>
Date: Mon, 13 Mar 2017 09:14:16 +0100

> I seem to remember that the advice simply is no longer working since JSON was extended from 4627 to 7159.  Instead of trying to come up with an updated algorithm, the WG recognized that this is not…

So the changes in RFC 7159 allow top-level strings, so we can't rely on 
the first *two* characters being US-ASCII. But we *can* rely on the 
first one being US-ASCII, no?

So the following should still be correct:

>    Since the first character of a JSON text will always be an ASCII
>    character [RFC0020], it is possible to determine whether an octet
>            00 xx xx xx  UTF-16BE
>            xx 00 xx xx  UTF-16LE

Best regards, Julian

---

From: Nico Williams <[email-masked]>
Date: Mon, 13 Mar 2017 13:15:34 -0500

On Mon, Mar 13, 2017 at 09:14:16AM +0100, Julian Reschke wrote:
> So the changes in RFC 7159 allow top-level strings, so we can't rely on the
> first *two* characters being US-ASCII. But we *can* rely on the first one
> being US-ASCII, no?

Correct.

If one OR two bytes of the first four are NULs, then the encoding is

> >   at the pattern of nulls in the first four octets.

Count the number of NULs in the first four bytes:

 - if zero -> UTF-8
 - if one or two -> UTF-16
 - if three -> UTF-32

Nico
--