Skip to content

Sanitiza HTML/JavaScript nos campos TextField (stored XSS) - #3846

Open
edwardoliveira wants to merge 1 commit into
3.1.xfrom
hotfix/sanitize-textfield-xss
Open

Sanitiza HTML/JavaScript nos campos TextField (stored XSS)#3846
edwardoliveira wants to merge 1 commit into
3.1.xfrom
hotfix/sanitize-textfield-xss

Conversation

@edwardoliveira

Copy link
Copy Markdown
Contributor

Problema

Os ~71 campos models.TextField do SAPL guardavam texto livre sem nenhuma sanitização, e a camada de renderização os tratava como HTML confiável em 142 pontos (|safe, {% autoescape off %}).

Resultado: stored XSS. Um usuário com permissão de edição grava <script> em TramitacaoAdministrativo.texto, MateriaLegislativa.ementa, Parlamentar.biografia etc. e o código executa no navegador de qualquer pessoa que abra a tela — inclusive nas páginas públicas (acompanhamento_materia, acompanhamento_documento, parlamentar_perfil_publico, resumo de sessão).

Dois agravantes encontrados durante a análise:

  • O sink principal não é um template, é uma função: get_field_display() (sapl/crispy_layout_mixin.py) monta o HTML de todo TextField e é consumida com |safe por crud/detail.html, crud/detail_detail.html e crud/list.html — praticamente toda tela de listagem/detalhe do sistema.
  • Cinco |safe estavam dentro de <textarea> (telas de votação), onde </textarea><script> escapa do contexto.

Solução

Duas camadas, ambas chamando a mesma função idempotente — é essa propriedade que permite aplicar as duas sem que os efeitos se acumulem:

Camada Onde Cobre
Entrada receiver pre_save global em sapl/base/receivers.py forms, API do drfautoapi, admin e shell
Saída get_field_display() + novo filtro |sanitize as linhas gravadas antes desta mudança

Não há migração de dados: as linhas existentes ficam neutralizadas na renderização e são limpas quando o registro for salvo de novo.

Políticas por campo (sapl/sanitize.py)

  • plain (padrão para todo TextField) — remove toda a marcação e preserva o texto. <b>oi</b> vira oi; < e & saem corretamente codificados.
  • rich — allowlist para os campos editados no TinyMCE: ExpedienteSessao.conteudo, OcorrenciaSessao.conteudo, ConsideracoesFinais.conteudo, Dispositivo.texto/texto_atualizador e os fragmentos *_html de TipoDispositivo.
  • exemptLexmlProvedor.xml (é XML, já escapado em pretty_xml) e TipoTextoArticulado.rodape_global (vai para um content: de CSS).

Links continuam funcionando

Entrada Saída
<a href="https://camara.gov.br">Portal</a> inalterado
<a href="/materia/123">Matéria</a> inalterado
<a href="javascript:alert(1)">clique</a> <a>clique</a> — texto mantido, URL descartada
<a href="#" onclick="steal()">x</a> <a href="#">x</a>
<script>…</script> removido junto com o conteúdo

target="_blank" e o style="text-align: …" dos botões de alinhamento do TinyMCE estão na allowlist de propósito, para não regredir o conteúdo já cadastrado. O rel="noopener noreferrer" passa a ser escrito automaticamente nos links.

Também corrigido

  • 5 |safe dentro de <textarea> (sessao/votacao/*).
  • 5 |striptags|safe — a documentação do Django avisa que striptags não garante HTML seguro, e o |safe desligava o autoescape sobre a sobra.
  • 3 {% autoescape off %} nos relatórios de matérias/normas por autor.

Dependência

nh3==0.2.22 (binding Rust do ammonia, sem dependências Python, wheels manylinux/macOS para cp312 — a imagem é python:3.12-slim-bookworm). É a substituta recomendada pelo bleach, hoje em modo de manutenção.

Testes

sapl/base/tests/test_sanitize.py — 23 testes, todos passando. Cobrem a idempotência das duas políticas, a preservação de links/formatação nos campos ricos, a remoção de script/on*/javascript:, o comportamento do pre_save, as isenções, e a proteção de uma linha legada gravada direto via .update().

Suíte completa: 22 falhas antes, 22 falhas depois, os mesmos testes — nenhuma regressão. (As falhas pré-existentes vêm de test_urls.py, redireciona_urls e forms de casa legislativa/audiência, sem relação com esta mudança.)

flake8 nos arquivos novos está limpo; nos arquivos tocados a contagem de violações pré-existentes continua idêntica (48 → 48).

Verificação manual sugerida

  1. Gravar <script>alert(1)</script>Teste em TramitacaoAdministrativo.texto e abrir o detalhe do documento e o filtro de matérias.
  2. Inserir um link pelo TinyMCE num expediente, salvar e reabrir — deve continuar clicável e abrindo em nova aba.
  3. Usar o botão code do TinyMCE para colar <a href="javascript:alert(1)">x</a><script>alert(2)</script> e conferir o resumo público da sessão.
  4. Abrir um expediente pré-existente com centralização e tabela e confirmar que está idêntico.
  5. Gerar a ata e a pauta em PDF da mesma sessão.
  6. Abrir e editar um texto articulado com dispositivos alterados.

Fora de escopo

  • relatorios/views.py e painel/views.py chamam html.unescape() em ementa/observacao/conteudo. Não é vetor de XSS (o destino é PDF/painel), mas revela que essas colunas já contêm entidades HTML hoje.
  • relatorios/base_relatorio.html interpola rodape dentro de um content: de CSS — contexto diferente, correção diferente.
  • O plugin code do TinyMCE expõe uma caixa de HTML bruto e é o caminho de injeção mais direto do sistema; desabilitá-lo reduziria a superfície, mas mexe no build do frontend.
  • sapl/crud/tests/test_base.py está com erro de sintaxe (import truncado) desde d08fb9e e não foi tocado aqui.

🤖 Generated with Claude Code

Os ~71 campos models.TextField do SAPL guardavam texto livre sem nenhuma
sanitização, e a renderização os tratava como HTML confiável em 142 pontos
(|safe, {% autoescape off %}). Qualquer usuário com permissão de edição
conseguia gravar <script> num campo e executá-lo no navegador de quem
abrisse a tela, inclusive nas páginas públicas (acompanhamento de matéria,
perfil público de parlamentar, resumo de sessão).

Duas camadas, ambas usando a mesma função idempotente para que não se
acumulem:

- Entrada: receiver pre_save global em sapl/base/receivers.py, cobrindo
  forms, a API do drfautoapi, o admin e o shell num único ponto.
- Saída: get_field_display() em sapl/crispy_layout_mixin.py — que monta o
  HTML de todo TextField para as telas de list/detail do CRUD — mais o
  novo filtro |sanitize nos templates. Protege as linhas gravadas antes
  desta mudança, já que não há backfill.

Políticas por campo, em sapl/sanitize.py:

- plain (padrão): remove toda a marcação e preserva o texto.
- rich: allowlist para os campos editados no TinyMCE (conteúdo de
  expediente, ocorrências, considerações finais e texto de dispositivo).
  Links continuam funcionando, com target e o alinhamento do editor
  preservados; javascript:, data:, on* e <script> são descartados.
- exempt: LexmlProvedor.xml e TipoTextoArticulado.rodape_global, que não
  são HTML.

Corrige também cinco |safe dentro de <textarea>, onde
</textarea><script> escapava do contexto, e cinco |striptags|safe — a
documentação do Django avisa que striptags não garante HTML seguro.

Sem migração de dados: as linhas existentes são neutralizadas na
renderização e limpas quando o registro for salvo de novo.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread sapl/sanitize.py

return nh3.clean(
value,
tags=set(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bloqueador — &, < e > passam a aparecer escapados na tela e na API.

nh3.clean(tags=set()) não reduz a texto puro, ele codifica em HTML. Como o pre_save grava o resultado no banco, os ~49 pontos de renderização que perderam o |safe escapam de novo:

digitado : CONCEDE TITULO A EMPRESA ALFA & BETA LTDA e fixa prazo < 30 dias
no banco : ...ALFA &amp; BETA LTDA e fixa prazo &lt; 30 dias
na tela  : ...ALFA &amp;amp; BETA ... prazo &amp;lt; 30 dias

O usuário lê literalmente &amp; e &lt;. Atinge acompanhamento_materia, resumo_detail_materia, search.html, os filtros de matéria/norma/documento, as telas de votação e a pauta. & é comum em ementa ("EMPRESA X & CIA").

Dois agravantes:

  • O get_field_display (que continua com |safe) renderiza a mesma linha corretamente, então os dois caminhos de renderização passam a discordar entre si.
  • A API também: /api/materia/materialegislativa/ devolve ALFA &amp; BETA dentro do JSON, onde não existe contexto HTML nenhum.

Sugestão: para campo de texto puro o banco deveria guardar texto puro mesmo (remover marcação e desescapar), deixando o escape para a renderização. Nesse caso o get_field_display precisa aplicar escape(), já que a saída dele é consumida com |safe.

</div>
<div class="row">
<div class="col-md-12"><b>Ementa:</b> {{materia.ementa|safe}}</div>
<div class="col-md-12"><b>Ementa:</b> {{materia.ementa}}</div>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sintoma do problema apontado em sapl/sanitize.py: como o valor já vem HTML-codificado do banco, o autoescape do template codifica de novo e esta página pública exibe &amp; e &lt; literais.

Verificado renderizando esta linha com o registro real:

<b>Ementa:</b> CONCEDE TITULO A EMPRESA ALFA &amp;amp; BETA LTDA e fixa prazo &amp;lt; 30 dias

Mesmo efeito em resumo_detail_materia.html, search/search.html, materialegislativa_filter.html, normajuridica_filter.html, documentoadministrativo_filter.html e nas telas de votação.

{% if e.conteudo %}
<b>{{ e.tipo }}</b>:
{{ e.conteudo|striptags|safe }}
{{ e.conteudo|striptags }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bloqueador — a ata passa a mostrar &nbsp; e &amp; literais.

striptags remove as tags mas deixa as entidades codificadas; sem o |safe elas escapam de novo. Confirmado em /sessao/1/resumo_ata:

<b>Oficios</b>: Ofício nº 12&amp;nbsp;–&amp;nbsp;lido em plenário &amp;amp; arquivado.

Conteúdo de TinyMCE é cheio de &nbsp; e de acentos em entidade, então isso aparece em praticamente toda ata. Vale para os 5 blocos alterados (blocos_ata/ e relatorios/blocos_sessao_plenaria/), que alimentam a ata e o PDF — documentos legais.

Trocar |safe por nada aqui não é ganho de segurança real: a doc do Django avisa que striptags não garante HTML seguro, mas a correção certa é renderizar o campo rico com |sanitize (ou desescapar depois do striptags), não escapar duas vezes.

Comment thread sapl/sanitize.py
# Os de sessao e compilacao.Dispositivo são editados no TinyMCE; os de
# compilacao.TipoDispositivo são fragmentos de template configurados por
# administradores.
RICH_TEXT_FIELDS = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bloqueador — faltam 4 campos de editor rico, o que causa perda permanente de HTML.

initTextRichEditor('#texto-rico') (frontend/src/__global/main.js:32) liga o TinyMCE globalmente em:

campo form
parlamentares.Parlamentar.biografia parlamentares/forms.py:216, :309
base.CasaLegislativa.informacao_geral base/forms.py:917
norma.NormaRelacionada.resumo norma/forms.py:423
parlamentares.Legislatura.observacao parlamentares/forms.py:161

Nenhum está em RICH_TEXT_FIELDS, então caem em plain. Salvando um Parlamentar numa instância real:

antes : <p>Eleito em 2020.</p><p>Advogado pela <a href="https://oab.org.br">OAB</a>.</p>
depois: Eleito em 2020.Advogado pela OAB.

Parágrafos fundidos e link destruído, sem volta. E como o get_field_display também achata esses campos na renderização, o conteúdo já cadastrado quebra na tela antes mesmo de alguém salvar.

(compilacao.TipoTextoArticulado.rodape_global também usa #texto-rico, mas esse está corretamente em SANITIZE_EXEMPT_MODELS.)

{% endif %}

<div class="dtxt" id="d{% if not dpt.dispositivo_subsequente_id and dpt.dispositivo_substituido_id %}a{% endif %}{{dpt.pk}}" pks="{{dpt.dispositivo_substituido_id|default:''}}" pk="{{dpt.pk}}">{{ dpt.tipo_dispositivo.texto_prefixo_html|safe }}{%if dpt.texto %}{{ dpt.texto|safe }}{%else%}{%if not dpt.tipo_dispositivo.dispositivo_de_articulacao %}&nbsp;{% endif %}{% endif %}</div>
<div class="dtxt" id="d{% if not dpt.dispositivo_subsequente_id and dpt.dispositivo_substituido_id %}a{% endif %}{{dpt.pk}}" pks="{{dpt.dispositivo_substituido_id|default:''}}" pk="{{dpt.pk}}">{{ dpt.tipo_dispositivo.texto_prefixo_html|safe }}{%if dpt.texto %}{{ dpt.texto|sanitize }}{%else%}{%if not dpt.tipo_dispositivo.dispositivo_de_articulacao %}&nbsp;{% endif %}{% endif %}</div>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A camada de saída ficou incompleta no compilacao. Aqui dpt.texto virou |sanitize, mas Dispositivo.texto / TipoDispositivo.*_html continuam com |safe em:

  • compilacao/dispositivo_form_search_fragment_child.html
  • compilacao/text_edit_blocoalteracao.html
  • compilacao/text_notificacoes.html
  • compilacao/layout/dispositivo_radio.html
  • compilacao/layout/dispositivo_checkbox.html

(nesta própria linha o texto_prefixo_html|safe também ficou.)

As linhas gravadas antes do pre_save seguem exploráveis por esses caminhos, o que contraria a proposta de "a camada de saída protege as linhas legadas". São telas autenticadas, então o risco é menor que o das páginas públicas, mas a cobertura fica inconsistente.

Fora do compilacao a cobertura dos sinks está completa — conferi que não sobrou nenhum |safe sobre campo de modelo.

Comment thread sapl/base/receivers.py
Cobre forms, a API do drfautoapi, o admin e o shell num único ponto.
Ver sapl.sanitize para as políticas por campo.
"""
if sender._meta.app_label not in SAPL_APP_LABELS:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O receiver não ignora raw=True. Por convenção, receivers de pre_save pulam quando kwargs.get('raw'), porque aí o objeto vem de loaddata e deve ser gravado literalmente.

Como o sinal fica conectado durante o migrate (decisão deliberada, pelo comentário logo abaixo), as fixtures carregadas dentro de migrations acabam reescritas — por exemplo materia/migrations/0013_adiciona_status_tramitacao.py, parlamentares/migrations/0044_adiciona_cargos_mesa.py e norma/migrations/0016_*.

Na prática essas fixtures são descrições simples, então o impacto hoje é pequeno; mas é barato blindar:

if kwargs.get('raw'):
    return

Comment thread sapl/sanitize.py
}

# Modelos cujos TextField não devem ser tocados em hipótese alguma.
SANITIZE_EXEMPT_MODELS = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) A isenção é por modelo, não por campo, enquanto RICH_TEXT_FIELDS é por campo. Hoje é inofensivo — conferi que LexmlProvedor e TipoTextoArticulado têm exatamente um TextField cada —, mas basta alguém adicionar outro TextField a esses modelos para ele ficar sem sanitização nenhuma, silenciosamente.

Usar a mesma forma de RICH_TEXT_FIELDS ({'lexml.LexmlProvedor': {'xml'}, ...}) deixaria as duas tabelas consistentes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants