Skip to content

ci: add linter task "ban unicode" to protect against malicious unicode #9801

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SomberNight
Copy link
Member

@f321x and I talked about how a malicious PR could try to add invisible unicode characters to the code and perhaps introduce backdoors/vulnerabilities that way. I think if we can add simple protection against such trickery, it might be worthwhile. I think the approach here is simple enough.


This new ban_unicode.py script scans the whole codebase for unicode characters and errors if it finds any, unless the character is specifically whitelisted. We can run it on the CI, its runtime is only ~2-3 seconds (+20 seconds for CI task overhead).

The motivation is to protect against homoglyph attacks, invisible unicode characters, bidirectional and other control characters, and other malicious unicode usage.

Given that we mostly expect to use ASCII characters in the source code, the most robust and generic fix seems to be to just ban all unicode usage. We only rarely use unicode characters, e.g. for ASCII drawings or in the name of a copyright-holder person. Every time we introduce a new usage (which has historically been very rare), we can just add the relevant characters to the whitelist.

see https://trojansource.codes/ :

Compilers, interpreters, and build pipelines supporting Unicode should throw errors or warnings
for unterminated bidirectional control characters in comments or string literals,
and for identifiers with mixed-script confusable characters.
Language specifications should formally disallow unterminated bidirectional
control characters in comments and string literals.
Code editors and repository frontends should make bidirectional control characters
and mixed-script confusable characters perceptible with visual symbols or warnings.

also https://github.com/maltfield/detect-malicious-unicode

This script scans the whole codebase for unicode characters and
errors if it finds any, unless the character is specifically whitelisted.

The motivation is to protect against homoglyph attacks, invisible unicode characters,
bidirectional and other control characters, and other malicious unicode usage.

Given that we mostly expect to use ASCII characters in the source code,
the most robust and generic fix seems to be to just ban all unicode usage.

see https://trojansource.codes/ :
> Compilers, interpreters, and build pipelines supporting Unicode should throw errors or warnings
> for unterminated bidirectional control characters in comments or string literals,
> and for identifiers with mixed-script confusable characters.
> Language specifications should formally disallow unterminated bidirectional
> control characters in comments and string literals.
> Code editors and repository frontends should make bidirectional control characters
> and mixed-script confusable characters perceptible with visual symbols or warnings.

also https://github.com/maltfield/detect-malicious-unicode
@SomberNight
Copy link
Member Author

Note: currently this new CI task is failing with:

electrum/gui/fonts/PTMono.LICENSE:0. line='\ufeffCopyright (c) 2011, ParaType Ltd. (http://www.paratype.com/public),'. hex=0xfeff. char='\ufeff'

I have intentionally left that failure in there to showcase how a failure would look.
(it is due to a unicode byte-order-mark at the beginning of that file)

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.

1 participant