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

TOB-FUEL-25: Invisible code points are supported in Sway programs #5047

Closed
xgreenx opened this issue Aug 28, 2023 · 0 comments · Fixed by #5146
Closed

TOB-FUEL-25: Invisible code points are supported in Sway programs #5047

xgreenx opened this issue Aug 28, 2023 · 0 comments · Fixed by #5146
Assignees
Labels
audit-report Related to the audit report compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: parser Everything to do with the parser

Comments

@xgreenx
Copy link
Contributor

xgreenx commented Aug 28, 2023

Description

The current sway parser allows any Unicode character in strings or comments, which can include invisible bidirectional override characters. Using such characters can lead to differences between the code reviewed in a pull request and the compiled code.

Figure 25.1: Sway program that logs “different”

script;
use std::string::String;
fn main() {
    let a = String::from_ascii_str("fuel");
    let b = String::from_ascii_str("fuel"); // Same string again
    if a.as_bytes() == b.as_bytes() {
        log("same");
} else {
        log("different");
    }
}

By default, VSCode shows the Unicode characters (figure 25.2). Google Docs, JetBrains CLion and GitHub display the source code as in figure 25.1. GitHub and JetBrains point out that bidi characters are present. GitHub shows by now a warning if bidi characters are present (see figure 25.3).

Figure 25.2: Code rendered using a non-bidi aware editor.

image

Figure 25.3: GitHub can also reveal the bidi characters.

image

Exploit Scenario

Figure 25.2: Code rendered using a non-bidi aware editor.
Figure 25.3: GitHub can also reveal the bidi characters.
An attacker creates a program that includes a backdoor, but hides the introduced bug using bidi Unicode characters. Review systems might not warn reviewers about the presence of bidi characters.

Recommendations

Short term, reject the following code points: U+202A, U+202B, U+202C, U+202D, U+202E, U+2066, U+2067, U+2068, U+2069. This list might not be exhaustive. Therefore, consider disabling all non-ASCII characters in the Sway language.
Long term, consider introducing escape sequences if not already done, so users can still use bidirectional code points if there is a legitimate use case.

@xgreenx xgreenx added the audit-report Related to the audit report label Aug 28, 2023
@anton-trunov anton-trunov added compiler: parser Everything to do with the parser compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen labels Sep 6, 2023
IGI-111 added a commit that referenced this issue Sep 26, 2023
Forbid directional formatting characters from
[UAX #9](https://www.unicode.org/reports/tr9/#Directional_Formatting_Characters)
in literals to fix #5047.
This is similar to rustc's `text_direction_codepoint_in_literal` lint.
Such characters are already implicitly forbidden in other parts of the syntax.
IGI-111 added a commit that referenced this issue Sep 26, 2023
Forbid directional formatting characters from
[UAX #9](https://www.unicode.org/reports/tr9/#Directional_Formatting_Characters)
in literals to fix #5047.
This is similar to rustc's `text_direction_codepoint_in_literal` lint.
Such characters are already implicitly forbidden in other parts of the syntax.
IGI-111 added a commit that referenced this issue Sep 26, 2023
## Description
Forbid directional formatting characters from
[UAX
#9](https://www.unicode.org/reports/tr9/#Directional_Formatting_Characters)
in literals to fix #5047.
This is similar to rustc's `text_direction_codepoint_in_literal` lint.
Such characters are already implicitly forbidden in other parts of the
syntax.


## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
@IGI-111 IGI-111 self-assigned this Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit-report Related to the audit report compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: parser Everything to do with the parser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants