wezterm: unbundle vendored libraries

[thiagokokada]

Wezterm ships with an outdated version of zlib (1.2.11) that has some known security issues:

• CVE-2018-25032
• CVE-2022-37434
• CVE-2023-45853

In general we should avoid shipping bundled libraries, so this also unbundles the other libraries since upstream basically uses upstream libraries without patches.

See: wezterm/wezterm#6540.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[thiagokokada]

CC @SuperSandro2000 for review.

[thiagokokada]

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 370353

---

aarch64-darwin

✅ 1 package built:

• wezterm

[thiagokokada]

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 370353

---

x86_64-linux

✅ 1 package built:

• wezterm

[thiagokokada]

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 370353

---

x86_64-darwin

✅ 1 package built:

• wezterm

[emilazy]

We should really make it link against our built packages normally rather than just extracting their source code if at all possible.

[thiagokokada]

I have zero experience in Rust to do so, but yes this would be the correct approach.

[SuperSandro2000]

Not sure if we should do that when there are files that are generated against the source code https://github.com/wez/wezterm/blob/main/deps/harfbuzz/src/lib.rs

[thiagokokada]

Upstream just updated all libraries to their last version when I asked them, so it seems fine.

[SuperSandro2000]

https://github.com/wez/wezterm/tree/main/deps/freetype seems to be not updated that often

[emilazy]

Looks like 2.13 is just new enough to dodge a bunch of really nasty vulnerabilities.

[emilazy]

I think we kind of just have to set knownVulnerabilities while working with upstream to get a proper way of using system libraries here (linking directly to them, doing bindgen at build time based on system headers rather than checking in the output, etc.). (Ideally upstream would also update their vendored copies too, of course…)

[thiagokokada]

I think we kind of just have to set knownVulnerabilities while working with upstream to get a proper way of using system libraries here (linking directly to them, doing bindgen at build time based on system headers rather than checking in the output, etc.). (Ideally upstream would also update their vendored copies too, of course…)

I don't think setting knownVulnerabilities is necessary here unless unbundling the included libraries stops working for some reason, but I definitely think someone needs to work upstream to link with system libraries like other Rust packages do. It is just not going to be me because I barely know Rust.

[SuperSandro2000]

We are not going to set knownVulnerabilities as long as hydra is not building the package! Wezterm takes ages to compile (30+ minutes on my laptop) and that will make the experience miserable for everyone! If we would just build vulnerable packages I would not care that much but that idea was heavily desputes for reasons I totally do not agree with.

IMO I have my doubts that those CVEs are actually exploitable in wezterm. I need to double check for what zlib is used but the font rendering libraries are only parsing fonts set by the user and not untrusted user input, so the risk is acceptable IMO.

[SuperSandro2000]

but I definitely think someone needs to work upstream to link with system libraries like other Rust packages do.

The problem here is that rust bindings are generated against them. They might be compatible with the next 4 updates and with the 5th we get incomprehensible linking and compiler errors I have zero capacity to work on or fix within a reasonable timeframe. Crates ending with -sys have the same problem but usually seem to work good enough across updates.

I mean sure, we can go that route and pray for the best but as soon as there is any issue with compiling against a newer library, I can only offer to remove the code again as I am missing time and skills to adequately fix those issues.

[emilazy]

We are not going to set knownVulnerabilities as long as hydra is not building the package!

This is not how it works. I understand your strong feelings on the matter but the consensus was pretty clearly against Hydra building insecure packages in #351429 and knownVulnerabilities is there for a reason. Packages with known vulnerabilities must be marked or we are doing a disservice to our users and individual package maintainers cannot override our project‐wide security practices.

If we can demonstrate that the vulnerabilities are not exploitable then that’s another thing, but that is a high bar to clear and many vulnerabilities have been incorrectly claimed to have no practical impact in the past.

I agree that source‐based unbundling is not a good solution when there is checked‐in pregenerated binding code that may not be compatible with newer versions. Using src also means that we don’t apply any patches from our derivations, even though those may include critical security fixes. I think the best approach here is to stick with the vendored libraries for now but

1. ask upstream to update them (they have already done that for some libraries, but e.g. FreeType is still behind); and
2. ask them to come up with a way for distros to use system‐provided libraries.

[thiagokokada]

I talked with Wez in Matrix and he said he will update the embedded freetype libraries. Also the only reason there is a fork is because Freetype repo in GitLab was slow to clone from GitHub and the GitHub fork was out of date (this doesn't seem to be an issue anymore though).

There is no plan to support system libraries though, so I think the best thing for now will be to switch to unstable branch for Wezterm. Once this happens I can open a PR switching to unstable, does this looks like a good solution @SuperSandro2000 @emilazy ?

[Mic92]

I think it's reasonable to expect that fonts can be trusted. We also trust PATH and LD_LIBRARY_PATH to not contain malicious code.

[thiagokokada]

I think it's reasonable to expect that fonts can be trusted. We also trust PATH and LD_LIBRARY_PATH to not contain malicious code.

Remember that I am not necessary talking about freetype here but zlib that upstream shipped with a 7 year old version until recently: https://github.com/madler/zlib/releases/tag/v1.2.11. And the main point is that it is difficult to pinpoint if the usage of zlib is safe inside the program (also it is difficult to pinpoint if the usage of freetype here is safe or not).

This is why my suggestion to switch to unstable builds for wezterm, now that upstream upgraded all bundled libraries (it is still using harfbuzz 9.0.0, but AFAIK no known issues here).

[lucasew]

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 370353

---

x86_64-linux

✅ 1 package built:

• wezterm

[Mic92]

Yes. Upstream seems to have bumped their bundling: wezterm/wezterm@0eaf755 we should just switch to unstable in this case. They even switched away from the fork.

[thiagokokada]

Replacement PR: #370838.