Accessibility strings would be vulnerable to xss

[jonathanolson]

If these are ever made translatable, or could change with a query parameter.

In JohnTravoltageA11yStrings, replace arrowKeysMoveFootString's value to:

'<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVQIW2NkYGD4DwABCQEBtxmN7wAAAABJRU5ErkJggg==" onload="window.location.href=atob(\'aHR0cHM6Ly93d3cueW91dHViZS5jb20vd2F0Y2g/dj1kUXc0dzlXZ1hjUQ==\')" />'

to reproduce.

We should have a better way to mark up bold/non-bold text, or we could use improved validation.

@mattpen, in your investigation with the OWASP validator, is that something we could use in rosetta to prevent this type of attack vector? (I'd really like to allow the current usage, as it's quite flexible for the translator).

Code review for #128

[jonathanolson]

Also @ariel-phet, I don't recall the timeline or chance of having accessible strings be translated (this and some other issues like handling long strings would only apply if we plan on doing this in the future at some time).

Is it valuable to create review items anticipating translation, or should we plan to leave related improvements until that point (like XSS/layout).

[mattpen]

The OWASP validator I looked at was a Java library. I'll investigate if a JavaScript library exists.

Alternatively, is this something that could be prevented with CSP, or do these types of tags need to be allowed elsewhere?

[jonathanolson]

Good point. Can CSP prevent execution from setting element.innerHTML? We'd also need to ensure that type of behavior wasn't needed elsewhere in sims.

[mattpen]

@jonathonolson - I misunderstood how these strings were being used, I don't think CSP will be viable. Anything that would block this text would also block sim code from running. I thought you were referring to alt attributes for images.

I couldn't find a widely trusted validation library for Node after a quick search. According to OWASP, Microsoft provides one for .NET framework and OWASP provides one for Java projects. We could potentially use the OWASP project but would need to build a new service to use it rather than integrating it directly in rosetta.

[ariel-phet]

@jonathanolson yes it is valuable to create review items anticipating translation.

Although there is not yet a specific timeline, updating the translation utility to allow translation of a11y strings is on the horizon.

[mattpen]

@jonathanolson - Let me know if you want help building a validator service. I'm removing my assignment for now.

[jessegreenberg]

Thanks for working on this @jonathanolson and @mattpen. This issue is not specific to this sim, would you mind if I move the ticket to rosetta?

[mattpen]

That sounds ok to me.

[jessegreenberg]

I opened phetsims/rosetta#146 and moved all comments to that issue. Closing.