reconciliation-api / specs

Specifications of the reconciliation API
https://reconciliation-api.github.io/specs/draft/
31 stars 9 forks source link

XSS vulnerability with flyouts #50

Closed wetneb closed 4 years ago

wetneb commented 4 years ago

As the specs currently note, the flyout services are dangerous because they let reconciliation endpoints return arbitrary HTML that clients are supposed to render on their side. But that HTML could potentially contain JS code, leading to XSS attacks.

OpenRefine issue: https://github.com/OpenRefine/OpenRefine/issues/3141

I would propose to drop flyout services:

workergnome commented 4 years ago

As a potential counter-argument, we include a small visualization of the AAT and TGN hierarchy in our fly-outs, which would be something very difficult to provide as description. Not a dealbreaker, of course, but it does give us potential that's not immediately replicable with just a textual description.

Would it make sense to whitelist tags in this area? I know that's something IIIF, for instance, has used to deal with this problem.

wetneb commented 4 years ago

Thanks, that makes sense! Out of curiosity, do you have an example to share, or is the service private?

The public Getty service does not use flyouts at the moment, it only uses previews (a different service, which I am not proposing to drop).

tfmorris commented 4 years ago

Can you explain "poor user experience"? It's not at all obvious to me, since I see them as the key way that users get the information that they need to confirm that a reconciliation candidate is the correct match.

In the original design, they were used to display Freebase Suggest widgets, wish were pretty similar to what one might see today in a Google Knowledge Graph "card" on the right side of search results. Things they might include would be an image, a "notable" type (ie a type which would ideally be most meaningful and distinguishing to a user), a list of types (with very common types filtered out), the name (obviously), a brief description. I can't remember if they did, but you could certainly imagine them including a few key facts which would vary by type like birth/death dates & occupation for people, office & tenure for politicians, etc.

The client is free to (and should) filter the HTML. If you wanted to get rid of the HTML, you could define a simple JSON structure with image, description, and list of label/value pairs that the client could render as it sees fit, but it would be more work to understand the needs of the reconciliation services and spec out something that would meet them.

tfmorris commented 4 years ago

Here's shot of what the flyout looked like (in an autocomplete context, but the widget was the same). https://developers.google.com/freebase/v1/search-widget Screen Shot 2020-08-30 at 2 23 21 PM

wetneb commented 4 years ago

By poor user experience I mean that we are stacking up UI layers. In Vladimir Alexiev's own words (https://github.com/OpenRefine/OpenRefine/issues/2334#issuecomment-591882176):

A popout over a dropdown over a modal dialog ?!?! What UX horror is this?

That being said, your screenshot is nice. Can you think of a reason why those flyouts should be different from previews? We could just re-use the preview service (so, with an <iframe>) there.

The client is free to (and should) filter the HTML.

The specs could specify which tags services can reliably include, otherwise it is hard for services to know what they can count on.

wetneb commented 4 years ago

This is a duplicate of #42, sorry. Let's continue the discussion there…