This will tentatively be scheduled for review during the next quarter (2024-01-01 through 2024-03-31).

Hey @Catrope - Given the security-review-related work that was already performed by you in T346097, I'm wondering if there's much left for our team to do here. We could likely run a few more automated tools and dig a bit deeper into the vendor code with some manual pentesting, but that would likely be the extent of what we could accommodate for the floating-ui codebase.

In T349569#9448365, @sbassett wrote:

Hey @Catrope - Given the security-review-related work that was already performed by you in T346097, I'm wondering if there's much left for our team to do here. We could likely run a few more automated tools and dig a bit deeper into the vendor code with some manual pentesting, but that would likely be the extent of what we could accommodate for the floating-ui codebase.

(Apologies for the late response, I was out sick last week)

I think you're right that there's not much else to do. I was mainly hoping that someone from the security team could look over the internal review that we did and double-check it (we tried to follow your procedures best we could, but we are not the Security team), and then formally respond with a risk assessment level ("low", "medium", etc.).

@Catrope - Ok, thanks. I'll plan to review your initial review work and go from there.

Hey @Catrope - Quick update: unfortunately, I've found a few issues with floating-ui during review. I'm going to make this task private and post them soon.

Security Review Summary - T349569 - 2024-03-14
Last commit reviewed: 800298e46b (@floating-ui/vue@1.0.2)

Summary

Overall, @floating-ui/vue@1.0.2 looks to be in decent shape save the reachable vulnerabilities via a few dev dependencies. We should work to mitigate these with Upstream which would likely reduce the overall risk-rating here. But for now, this has to be a medium risk, which will need to be mitigated or owned within our application security risk registry by a directory/c-level at the WMF.

General Security Information

Scorecard score
5.5 / 10 medium
(see raw output: P58819)

Vulnerable Packages - Production
Risk: None, (no production packages)

Vulnerable Packages - Development (via osv-scanner)
high

IMPORTANT: Unfortunately, semgrep supply chain found a few vulnerable dependencies that are reachable within floating-ui (see: P58820). It looks like many of these would be mitigated by upgrading to the latest version: @floating-ui/vue@1.0.6, but there is still a high, reachable vite vulnerability within v1.0.6. I'd note that they've also chosen to use pnpm as opposed to npm in v1.0.6.

Outdated Packages
As reported via (p)npm outdated:
(no explicit vulnerabilities reported, simply noting for completeness' sake.)
None. (though there are 32(!) in @floating-ui/vue@1.0.6)

Static Analysis Findings
Risk: low.
A handful of popular, FOSS static analysis tools were run against the @floating-ui/vue@1.0.2 codebase. A summary of these tools' output exists below:

1. semgrep with various custom and well-known policies (results: P58821). The reported issues were either false positives or part of floating-ui's bundled website code, so this is low risk for now.
2. semgrep with some basic JavaScript sink-finding rules. The report seems to have produced all false positive (which is commong for such a general scanning of code), (results: P58823). This is low risk for now.
3. bearer sast (results: P58822). The reported issues were mostly part of floating-ui's bundled website code. There were a couple of minor disclosure issues towards the end, but overall these results appear to be low risk.
4. scan.sh sast returned no results which were not low-risk or already found via other tooling. low risk
5. lockfile-lint returned no results. low risk
6. gitleaks returned no true positive results. low risk
7. git secrets returned no true positive results. low risk
8. whispers returned no results. low risk

HTTP and other protocol Leaks
Risk: None, (no credible leaks found).

Miscellanous Issues/Points of Discussion/Nits
Risk: None..

1. floating-ui bundles their website code within the same repository (/website). This isn't ideal, but seems mostly harmless and compartmentalized at this time.

In T349569#9643439, @sbassett wrote:

Overall, @floating-ui/vue@1.0.2 looks to be in decent shape save the reachable vulnerabilities via a few dev dependencies. We should work to mitigate these with Upstream which would likely reduce the overall risk-rating here. But for now, this has to be a medium risk, which will need to be mitigated or owned within our application security risk registry by a directory/c-level at the WMF.

Hopefully we can mitigate this and reduce this back to a "low" quickly.

IMPORTANT: Unfortunately, semgrep supply chain found a few vulnerable dependencies that are reachable within floating-ui (see: P58820). It looks like many of these would be mitigated by upgrading to the latest version: @floating-ui/vue@1.0.6, but there is still a high, reachable vite vulnerability within v1.0.6. I'd note that they've also chosen to use pnpm as opposed to npm in v1.0.6.

I've submitted a patch to upgrade to v1.0.6 here: https://gerrit.wikimedia.org/r/c/design/codex/+/1012793 . As far as the Vite vulnerability, my understanding is that Vite (and the other floating-ui devDependencies) are only used in floating-ui's own build process, which we don't run ourselves. In particular, the Vite vulnerability is specific to Vite's devserver, which we also don't run in the context of floating-ui (we do run it in Codex, using a more recent version of Vite -- that version is also vulnerable, so I've submitted a patch to upgrade Codex's version of Vite here) . We only consume the output of this build process, and manually review changes to it. Obviously we would prefer that upstream patch this, but do these things change your opinion on the severity and reachability of this vulnerability?

Adding more members of the Design-System-Team to CC so they can see this now-hidden task.

In T349569#9644504, @Catrope wrote:

I've submitted a patch to upgrade to v1.0.6 here: https://gerrit.wikimedia.org/r/c/design/codex/+/1012793.

+1'd

As far as the Vite vulnerability, my understanding is that Vite (and the other floating-ui devDependencies) are only used in floating-ui's own build process, which we don't run ourselves. In particular, the Vite vulnerability is specific to Vite's devserver, which we also don't run in the context of floating-ui... We only consume the output of this build process, and manually review changes to it. Obviously we would prefer that upstream patch this, but do these things change your opinion on the severity and reachability of this vulnerability?

It would be nice if Upstream could address the vite issue sooner than later. While it's likely not quite as severe for us, it is a reachable vulnerability that would theoretically allow an attacker another pathway to compromise the various build processes they use for releases of floating-ui, which as you note, we do consume. So there is some risk there, if perhaps, a bit difficult to exploit and not directly exploitable against our codebases.

(we do run it in Codex, using a more recent version of Vite -- that version is also vulnerable, so I've submitted a patch to upgrade Codex's version of Vite here).

Ok, I think you might have meant https://gerrit.wikimedia.org/r/c/design/codex/+/1012795, but yes, 4.5.2 fixes the vite vuln, so I've +1'd that as well.

If we're concerned about a vulnerability in Floating UI, would it be worthwhile to try and open a PR in that project that upgrades Vite from 5.0.2 to 5.0.12?

I don't know if that would be considered advertising a vulnerability, but a PR to bump the dep version with an explanation of why it's needed would hopefully be accepted upstream.

In T349569#9646732, @egardner wrote:

If we're concerned about a vulnerability in Floating UI, would it be worthwhile to try and open a PR in that project that upgrades Vite from 5.0.2 to 5.0.12?

I don't know if that would be considered advertising a vulnerability, but a PR to bump the dep version with an explanation of why it's needed would hopefully be accepted upstream.

It'd probably be best to follow their security policy first: https://github.com/floating-ui/floating-ui/security. And hope they are responsive.

In T349569#9646835, @sbassett wrote:

It'd probably be best to follow their security policy first: https://github.com/floating-ui/floating-ui/security. And hope they are responsive.

Oh, good catch. I can reach out to the developer at the email address provided and let him know about the Vite issue. I can report back here if he responds.

In T349569#9647538, @egardner wrote:

In T349569#9646835, @sbassett wrote:

It'd probably be best to follow their security policy first: https://github.com/floating-ui/floating-ui/security. And hope they are responsive.

Oh, good catch. I can reach out to the developer at the email address provided and let him know about the Vite issue. I can report back here if he responds.

Hey @egardner - any update on this? I'm hesitant to make this task public if we haven't yet filed an upstream security issue for this, since we're discussing it here. Thanks.

In T349569#9698664, @sbassett wrote:

In T349569#9647538, @egardner wrote:

In T349569#9646835, @sbassett wrote:

It'd probably be best to follow their security policy first: https://github.com/floating-ui/floating-ui/security. And hope they are responsive.

Oh, good catch. I can reach out to the developer at the email address provided and let him know about the Vite issue. I can report back here if he responds.

Hey @egardner - any update on this? I'm hesitant to make this task public if we haven't yet filed an upstream security issue for this, since we're discussing it here. Thanks.

Hey @sbassett – I reached out to the maintainer just before a 2-week stint of travel (which I am now back from). Sounds like he would welcome a PR but doesn't see this as a huge priority since the package in question is a dev dependency instead of a runtime one.

Now that I'm back, I can file a PR this week – I can post back here with a link once that's done.

In T349569#9699086, @egardner wrote:

Hey @sbassett – I reached out to the maintainer just before a 2-week stint of travel (which I am now back from). Sounds like he would welcome a PR but doesn't see this as a huge priority since the package in question is a dev dependency instead of a runtime one.

Interesting, given that semgrep supply chain found this to be a reachable vulnerability. It might be worth reiterating that point to them along with the sometimes-false assumption that dev dependency vulnerabilities result in no risk.

Now that I'm back, I can file a PR this week – I can post back here with a link once that's done.

Sounds good. Once that's filed, I think we can/should make this task public.

@sbassett the Inuka team is considering using floating-ui for Wikipedia-Preview. We would be using @floating-ui/dom, which @floating-ui/vue is based on. Is the review done here sufficient or do we need to go through another round? thanks

In T349569#9842909, @SBisson wrote:

@sbassett the Inuka team is considering using floating-ui for Wikipedia-Preview. We would be using @floating-ui/dom, which @floating-ui/vue is based on. Is the review done here sufficient or do we need to go through another round? thanks

I think this should be fine. @floating-ui/dom was never reviewed line-for-line (we aren't really resourced to do that for every external package we might use) but it's already a direct dependency of @floating-ui/vue and, transitively, codex.

Meanwhile I've just posted a PR to the floating-ui repo to fix the Vite vulnerability: https://github.com/floating-ui/floating-ui/pull/2920.

I'll update again when I get some feedback from the maintainers.