Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Garantir dados seguros em campos que usam markdown #228

Open
MatheusBuss opened this issue May 11, 2022 · 5 comments
Open

Garantir dados seguros em campos que usam markdown #228

MatheusBuss opened this issue May 11, 2022 · 5 comments
Labels
🔒 privacy / infosec Privacy and security related issues

Comments

@MatheusBuss
Copy link
Contributor

Descrição do Problema

Atualmente os campos que aceita markdown não estão sendo sanitizados.

Implementar códigos que garantam a sanitização de dados para que não estejamos vulneráveis a ataques de injeção.

Campos de interesse:

  • Descrição de usuário
  • Descrição e título da ação
  • Descrição do objetivo
  • Descrição de produtos
  • Descrição da comunidade
  • Comunicados
  • Memo da transferência
@MatheusBuss
Copy link
Contributor Author

@NeoVier can you look into this issue? We should check all fields that accept markdown

@henriquecbuss
Copy link
Member

Besides the ones listed on the issue description, these fields accept markdown:

  • Action verification instructions
  • Sponsorship "Thank you" message
  • Shop memo (I believe this is the same as regular transfers memos)

Also, action titles are plain text (not markdown)


From the frontend perspective, this isn't really a problem (I'm thinking of JS injection, I'm not sure if there's something else we should worry about from the frontend perspective), because:

  1. Elm straight up doesn't let us add script tags through text (it just shows the plain text <script> ... </script>) - this is valid for any tag, actually
  2. We use dillonkearns/elm-markdown to render markdown, and it allows us to use custom renderers. In our case, the only valid HTML tag is u (underline). If there is any other html tag in the markdown, it's just not rendered at all.

It's nice to keep in mind that, behind the scenes, we just wrap around quill (a rich text editor component) to receive input. That input is in a format that quill understands, which uses json. We have a module created by us that manually converts that to a markdown AST, and then to markdown before sending it to the backend, to make it easier to store and to use in other places (e.g. emails).

That means we could actually convert it to any format that's easier for the backend to parse, if we wish to

@lucca65 lucca65 added the 🔒 privacy / infosec Privacy and security related issues label May 13, 2022
@lucca65 lucca65 added this to the 🧹 Housekeeping milestone May 13, 2022
@MatheusBuss
Copy link
Contributor Author

MatheusBuss commented Jul 21, 2022

Fazendo uma pesquisa não sei se temos muito pra se preocupar.

Usamos o Ecto pra interagir com o banco de dados, que já possui proteções contra injeções de SQL [1, 2]. Então acredito que só precisamos continuar utilizando o Ecto.

Quanto a outras vulnerabilidades de injeção de código, tentei um pouco pelo GraphQl, mas não tive sucesso. Quem sabe podemos conversar melhor sobre outras vulnerabilidades @lucca65?

@lucca65
Copy link
Member

lucca65 commented Jul 21, 2022

bro da uma pesquisada mais a fundo sobre XSS, que é a vulnerabilidade que estamos buscando evitar

@lucca65
Copy link
Member

lucca65 commented Jul 29, 2022

Uma referência de como isso é feito em Python: https://github.com/Wenzil/mdx_bleach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔒 privacy / infosec Privacy and security related issues
Projects
Status: 🧊Waiting
Development

No branches or pull requests

3 participants