Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Upgrade to Bleach 1.5, to limit link schemes to an allowlist #2860

Open
cuibonobo opened this issue Feb 9, 2018 · 3 comments
Open

Upgrade to Bleach 1.5, to limit link schemes to an allowlist #2860

cuibonobo opened this issue Feb 9, 2018 · 3 comments
Labels
A-Email-Push Email notifications good first issue Good for newcomers P4 (OBSOLETE: use S- labels.) Okay backlog: will not schedule, will accept patches T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution

Comments

@cuibonobo
Copy link

cuibonobo commented Feb 9, 2018

The 1.5 version of Bleach made the allowed protocols configurable: http://bleach.readthedocs.io/en/latest/clean.html#allowed-protocols-protocols

Once the dependency is updated, the safe_markup function in the mailer can be updated and these lines can be uncommented:

# When bleach release a version with this option, we can specify schemes
# ALLOWED_SCHEMES = ["http", "https", "ftp", "mailto"]

@neilisfragile neilisfragile added z-p2 (Deprecated Label) z-feature (Deprecated Label) improvement and removed z-feature (Deprecated Label) labels Feb 12, 2018
@erikjohnston erikjohnston added enhancement Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution and removed improvement labels Jun 11, 2020
@MadLittleMods MadLittleMods added the T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. label May 19, 2022
@MadLittleMods
Copy link
Contributor

It looks like we're already using v2:

bleach = ">=2.1.0"

And we just have a slightly broader version defined in pyproject.toml (docs):

bleach = ">=1.4.3"

Can we bump the minimum version to 1.5 to make the proposed change? What determines our minimums? Do we just ask our packagers?

@MadLittleMods MadLittleMods added the A-Email-Push Email notifications label May 19, 2022
@DMRobertson
Copy link
Contributor

It looks like we're already using v2:

The bit of the lockfile you quoted refers to the dependencies of the readme-renderer package (whatever that is). We've actually locked 4.1.0:

synapse/poetry.lock

Lines 83 to 89 in 6ff99e3

[[package]]
name = "bleach"
version = "4.1.0"
description = "An easy safelist-based HTML-sanitizing tool."
category = "main"
optional = false
python-versions = ">=3.6"

but it might be easier to spot this by looking at the locked checksums:

synapse/poetry.lock

Lines 1618 to 1621 in 6ff99e3

bleach = [
{file = "bleach-4.1.0-py2.py3-none-any.whl", hash = "sha256:4d2651ab93271d1129ac9cbc679f524565cc8a1b791909c4a51eac4446a15994"},
{file = "bleach-4.1.0.tar.gz", hash = "sha256:0900d8b37eba61a802ee40ac0061f8c2b5dee29c1927dd1d233e075ebf5a71da"},
]

A note of caution: the lockfile only describes the contents of

  • our docker images
  • our Debian packages
  • anyone installing using the poetry lockfile

It doesn't cover:

  • third party images or packages
  • anyone installing from PyPI (pip install matrix-synapse).

What determines our minimums? Do we just ask our packagers?

Pretty much. I sometimes use pkgs.org as a santiy check too, e.g. here and here.

@DMRobertson DMRobertson added P4 (OBSOLETE: use S- labels.) Okay backlog: will not schedule, will accept patches and removed z-enhancement z-p2 (Deprecated Label) labels May 19, 2022
@DMRobertson
Copy link
Contributor

The packagers don't seem to have any objections.

If anyone wants to pick this up, the steps are:

  • Raise the lower bound on bleach in pyproject.toml
  • Run poetry lock --no-update
  • Uncomment the lines containing ALLOWED_SCHEMES
  • Ideally: add a test case (or otherwise) demonstrate that safe_markup makes use of the this schema allowlist.

@DMRobertson DMRobertson changed the title Upgrade to Bleach 1.5 Upgrade to Bleach 1.5, to limit link schemes to an allowlist May 19, 2022
@DMRobertson DMRobertson added the good first issue Good for newcomers label May 19, 2022
krish-bista added a commit to krish-bista/synapse that referenced this issue Jan 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Email-Push Email notifications good first issue Good for newcomers P4 (OBSOLETE: use S- labels.) Okay backlog: will not schedule, will accept patches T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution
Projects
None yet
Development

No branches or pull requests

5 participants