Potential LRS security issue around malicious LRP storing html in document APIs

[garemoko]

In case people missed the Google Groups issue: https://groups.google.com/a/adlnet.gov/forum/#!topic/xapi-spec/MTkkxIBY6zk

As mentioned there, I think it's possible to implement an LRS to avoid this issue (and also to implement one that's vulnerable). Generally we've said that security is out of scope of the spec but in this case the issue is quite particular to xAPI so I wonder if it's worth a mention, perhaps with some recommendations on how to avoid problems.

[rmlowe]

FWIW, in the LRS that I'm involved with I think we will restrict content types that can be stored based on a whitelist, as this seems to me the only reliable way to avoid the issue.

It's not at all clear to me that we can do this and still be conformant however. I'd like to see the spec make that explicit.

Concretely, I'd like the spec to say that an LRS MAY reject and/or modify documents for security reasons (so an LRS could reject all text/html documents, or sanitize them by removing scripting elements/attributes for example).

Perhaps it would make sense to include a whitelist of types that the LRS MUST accept without modification (e.g. application/json, application/xml, text/plain).

Happy to formulate a pull request if people think that's reasonable.

[garemoko]

Duplicating part of my response form Google Groups:

The LRS is allowed to reject pretty much any request on the basis of permissions, so could restrict fetching HTML documents to credentials that it's really really careful to ensure don't fall into the wrong hands. This is made a lot more explicit in 1.0.3 https://github.com/adlnet/xAPI-Spec/blob/1.0.3/xAPI-Communication.md#32-error-codes

[fugu13]

I made a response in the thread, noting the x-experience-api-version header requirement mitigates this for many sorts of requests.

After thinking about it a little longer, Wax currently plans to mitigate by overwriting with a content-type that doesn't lead to browser rendering in modern browsers all requests matching any of the following

• no x-experience-api-version header sent
• cookie auth

Additionally, we will also refuse to respond to requests with a referrer from anywhere in the document APIs on Wax

I don't see a spec issue for any of those; the first two definitely aren't spec issues, and the third is very narrowly tailored on something well outside the intended spec usage model. Even if a larger intervention was needed, I view security issue mitigation as an overriding concern, so long as any area of noncompliance is narrowly tailored to meet the security concern.

[garemoko]

@andyjohnson did this ever get discussed on a call? If we decide that this needs to be mentioned in the spec, it'd be good to include it in 1.0.3 if at all possible.

[andyjohnson]

We did not unfortunately - my fault. I'd say either way the best we are doing is descriptive language (a MAY requirement is also okay as functionally this is almost the same thing) at this point, which I think won't do any harm. Start readying what you would want to do but don't PR until the sweeping PRs are merged, please.

[garemoko]

Notes from the call yesterday:

Given the changing nature of the world and the craftiness of humans, it is likely that security issues might arise which have not been envisaged and tackled within this specification. LRSs MAY breach conformance requirements in order to prevent security issues. They SHOULD do this in as focussed and narrow an approach as possible. For example the specific security issue flagged up here.

@fugu13 to make a PR.

[andyjohnson]

I think this is the last PR we are waiting on! @fugu13 - I can take a shot at it if you don't have bandwidth.

[garemoko]

I'm gonna take a shot at this. Adding it to https://github.com/adlnet/xAPI-Spec/blob/1.0.3/xAPI-Communication.md#50-security