| Summary: | [Bugzilla] Code reviews show non-ASCII characters in patches as garbage | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||||
| Component: | New Bugs | Assignee: | Myles C. Maxfield <mmaxfield> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ap, darin, mitz, webkit-bug-importer, wilander | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=75394 https://bugs.webkit.org/show_bug.cgi?id=45760 |
||||||||
| Attachments: |
|
||||||||
|
Description
Myles C. Maxfield
2022-03-31 13:35:59 PDT
Created attachment 456274 [details]
Patch
Created attachment 456275 [details]
Patch
Committed r292170 (249077@trunk): <https://commits.webkit.org/249077@trunk> I just realized that there is a downside to this change. Now our review page no longer serves as a defense against attacks like https://trojansource.codes. This seems fairly serious, perhaps we should undo it? If we (the WebKit team) decide that we do need to take additional actions here (a conclusion which I don’t think is foregone), the action shouldn’t be to revert this patch thereby relying on accidental behavior. If we want to show certain characters as visible, we should do so by intentionally writing code to do it. If we do want a mitigation, we can surely find a middle ground where some Unicode characters are replaced but others aren’t. (In reply to mitz from comment #5) > Is this a duplicate of bug 75394? It looks to me like “yes” but if that bug is still reproducing then maybe not? It doesn't seem worth the effort to develop any non-trivial mitigations, as pretty soon patch review will be on GitHub. But while it's here, it seems not great to create security risk. *** Bug 75394 has been marked as a duplicate of this bug. *** Does GitHub code review have mitigations for Trojan Source? Looks like GitHub displays a message, e.g https://github.com/nickboucher/trojan-source/blob/eaca01d01bfd588d805fbc711c39a1b3679ac484/Python/commenting-out.py |