Page MenuHomePhabricator

Application Security Review Request : Floating UI
Closed, ResolvedPublicSecurity

Description

Project Information

Description of the tool/project: Client-side JavaScript library for positioning a DOM element relative to another DOM element, and keeping those positions in sync as the user scrolls or resizes the viewport

Description of how the tool will be used at WMF: Floating UI is used inside Codex to position Menu components relative to the text input or dropdown they're related to. Floating UI is embedded in the Codex build, which allows us to only load the parts of the library we need. Codex uses the @floating-ui/vue package from NPM.

Dependencies
Floating UI's only runtime dependency is Vue (which is already a runtime dependency of Codex)

Has this project been reviewed before?
No

Working test environment
Run Codex locally (see instructions), or test the Lookup component on its documentation page.

Post-deployment
The Design-System-Team will continue to be responsible for Codex and for the use of Floating UI in Codex.

Details

Risk Rating
Low
Author Affiliation
WMF Product

Event Timeline

sbassett subscribed.

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

sbassett moved this task from Upcoming Quarter Planning Queue to In Progress on the secscrum board.
sbassett added a project: user-sbassett.
sbassett moved this task from Backlog to In Progress on the user-sbassett board.

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.

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.

sbassett set Security to Software security bug.Mar 19 2024, 6:22 PM
sbassett added a project: Security-Team.
sbassett changed the visibility from "Public (No Login Required)" to "Custom Policy".
sbassett changed the subtype of this task from "Task" to "Security Issue".
sbassett moved this task from Incoming to In Progress on the Security-Team board.
sbassett added a project: SecTeam-Processed.

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

Statistic/InfoValueRisk
Repositoryhttps://github.com/floating-ui/floating-ui none
Relevant tag/branch@floating-ui/vue@1.0.2 none
Last commit reviewed (if relevant)800298e46b none
Recent contributions to code (6 months)179 commit via 14 authors low
Active developers with > 10 commits3 medium
Current overall usage28k stars, 1.5k forks, 253 watchers low
Current open security issuesNone none
Methods for reporting security issueshttps://github.com/floating-ui/floating-ui/security low

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

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

Vulnerable Packages - Development (via osv-scanner)
*Risk: high*

VulnerabilityPackageServiceRemediationRisk
https://osv.dev/GHSA-67hx-6x53-jw92@babel/traverse@7.22.5osv[see advisory link][see advisory link]
https://osv.dev/GHSA-4q6p-r6v2-jvc5get-func-name@2.0.0osv[see advisory link][see advisory link]
https://osv.dev/GHSA-78xj-cgh5-2h22ip@1.1.8osv[see advisory link][see advisory link]
https://osv.dev/GHSA-7hpj-7hhx-2fgxmsgpackr@1.8.5osv[see advisory link][see advisory link]
https://osv.dev/GHSA-7hpj-7hhx-2fgxmsgpackr@1.9.5osv[see advisory link][see advisory link]
https://osv.dev/GHSA-7fh5-64p2-3v2jpostcss@package-lock.jsonosv[see advisory link][see advisory link]
https://osv.dev/GHSA-rxrc-rgv4-jpvxreact-devtools-core@4.27.8osv[see advisory link][see advisory link]
https://osv.dev/GHSA-c2qf-rxjj-qqgwsemver@5.7.1osv[see advisory link][see advisory link]
https://osv.dev/GHSA-c2qf-rxjj-qqgwsemver@6.3.0osv[see advisory link][see advisory link]
https://osv.dev/GHSA-c24v-8rfc-w8vwvite@4.3.9osv[see advisory link][see advisory link]

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.
sbassett changed the task status from Open to In Progress.Mar 19 2024, 6:24 PM
sbassett triaged this task as Medium priority.
sbassett moved this task from In Progress to Done on the user-sbassett board.
sbassett added subscribers: egardner, AnneT.

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.

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.

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.

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.

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.

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.

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 changed the visibility from "Custom Policy" to "Public (No Login Required)".
sbassett moved this task from In Progress to Watching on the Security-Team board.
sbassett moved this task from Waiting to Our Part Is Done on the secscrum board.