Accessibility strings would be vulnerable to XSS

[jessegreenberg]

From phetsims/john-travoltage#175. Copying discussion to rosetta because this isssue is not specific to john-travoltage.

@jonathanolson said:

If [a11y] strings 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).

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 said:

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 said:

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 said:

@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 said:

@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 said:

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

Assigning to @jonathanolson since phetsims/john-travoltage#175 was previously assigned to him.

[pixelzoom]

@jessegreenberg is this issue related to phetsims/tasks#917?

Was this boilerplate:

 if ( phet.chipper.queryParameters.stringTest === 'xss' ) {
   ...
  }

... added to all *A11Strings to address this issue?

[zepumph]

It is related to phetsims/tasks#917

Was this boilerplate: ... added to all *A11Strings to address this issue?

Yes that's right.

I'm going to close this issue because we are very close to getting rid of A11yStrings and moving the strings to the en json files, see #193 for base issue and phetsims/chipper#795 for next steps.