<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">Le 2 oct. 2017 à 21:40, Chris Lattner <<a href="mailto:clattner@nondot.org" class="">clattner@nondot.org</a>> a écrit :</div><div class=""><div class=""><br class=""><blockquote type="cite" class="">On Oct 2, 2017, at 1:13 AM, Félix Cloutier via swift-evolution <<a href="mailto:swift-evolution@swift.org" class="">swift-evolution@swift.org</a>> wrote:<br class=""><br class="">If you tried hard enough, you could probably create a variable that looks like it's shadowing one from an outer scope while it actually isn't, and use the two to confuse readers. This could trick people into thinking that some dangerous/backdoor code is actually good and safe, especially in the open-source world where you can't always trust your contributors.<br class=""><br class="">On one hand, other than the complexity of telling if two characters are lookalikes, I don't know why Αrray (GREEK CAPITAL LETTER ALPHA) and Array (LATIN CAPITAL LETTER A) should be considered different identifiers. On the other hand, I struggle to imagine the specifics of an exploit that uses that. You'd have to work pretty hard to assemble all the pieces of a backdoor in visually-similar variable names without arousing suspicion.<br class=""></blockquote><br class="">I don’t think this is something we have to try hard to avoid. It is true that some characters look similar, particularly in some fonts, but this isn’t new:<br class=""><br class=""> let a1 = 42<br class=""> let al = 12<br class=""> let b = al + a1 <br class=""></div></div></blockquote><div><br class=""></div><div>There is a fundamental difference between similar characters and characters that are meant to be visually identical. People judge the quality of a font by its Unicode support, and that means that only "low-quality" fonts would render, say, LATIN CAPITAL LETTER T and GREEK CAPITAL LETTER TAU differently.</div><br class=""><blockquote type="cite" class=""><div class=""><div class="">If there were real code that was maliciously shadowing to try to cause confusion, then you have a more serious problem on your hands than someone accidentally misunderstanding which one to use.<br class=""></div></div></blockquote><div><br class=""></div>I'm not sure I understand. If the "more serious problem" you're talking about is that your popular project is a valuable target to subvert, then there is no question that being backdoored would be more serious than people not reading your code right. I don't see how it pushes the problem out of scope, though.<br class=""><div><br class=""></div><div>As a security guy, I take my role of thinking about how anything can be abused very seriously. Backdoored open source projects turn up every now and then.</div><div><br class=""></div><div>This code is backdoored. I challenge you to spot the bug:</div><div><br class=""></div><div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; background-color: rgb(253, 246, 227);" class=""><div style="margin: 0px; font-stretch: normal; line-height: normal; color: rgb(101, 123, 131);" class=""><span style="color: #859901" class="">func</span> shellEscape(<span style="color: #859901" class="">_</span> args: [<span style="color: #b58901" class="">String</span>]) -> [<span style="color: #b58901" class="">String</span>]?</div><div style="margin: 0px; font-stretch: normal; line-height: normal; color: rgb(101, 123, 131);" class=""><span style="color: #859901" class="">func</span> isWhitelisted(<span style="color: #859901" class="">_</span> tool: <span style="color: #b58901" class="">String</span>) -> <span style="color: #b58901" class="">Bool</span></div><div style="margin: 0px; font-stretch: normal; font-size: 12px; line-height: normal; font-family: Helvetica; min-height: 14px;" class=""><br class=""></div><div style="margin: 0px; font-stretch: normal; line-height: normal; color: rgb(101, 123, 131);" class=""><div style="margin: 0px; font-stretch: normal; line-height: normal;" class=""><span style="color: #859901" class="">func</span> execute(externalTool: <span style="color: #b58901" class="">String</span>, parameters: [<span style="color: #b58901" class="">String</span>]) {</div><div style="margin: 0px; font-stretch: normal; line-height: normal;" class=""> <span style="color: #859901" class="">if</span> <span style="color: #6c71c4" class="">isWhitelisted</span>(externalTool), <span style="color: #859901" class="">let</span> pаrameters = <span style="color: #6c71c4" class="">shellEscape</span>(parameters) {</div><div style="margin: 0px; font-stretch: normal; line-height: normal;" class=""> print(<span style="color: #2aa198" class="">"Running tool </span>\<span style="color: #2aa198" class="">(</span>pаrameters[<span style="color: #2aa198" class="">0</span>]<span style="color: #2aa198" class="">)"</span>)<span style="color: #2aa198" class="">"</span></div><div style="margin: 0px; font-stretch: normal; line-height: normal;" class=""> system(parameters.joined(separator: <span style="color: #2aa198" class="">" "</span>))</div><div style="margin: 0px; font-stretch: normal; line-height: normal;" class=""> }</div><div style="margin: 0px; font-stretch: normal; line-height: normal;" class="">}</div></div></div></div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div class="">All I’m saying is that we shouldn’t complicate the design to solve this problem (IMO). If it falls out of the solution somehow (e.g. just disallow invisible characters) then that’s great of course!<br class=""></div></div></blockquote><div><br class=""></div><div>How did you identify the bug in the snippet from above? Is it practical enough that you would, for instance, recommend that the server group do that test on every PR that they receive going forward?</div><div><br class=""></div><div>I think that it's hard to build something meaningful without making it look suspicious. It's already kind of fishy that my shellEscape function returns an Optional, and people will eventually figure out that the parameters are not, in fact, shell-escaped. Still, I feel that it should be recognized that security is more than buffer overflows and integer overflows, and if there ever is an underhanded Swift code contest, that'll be my entry.</div></div><br class=""><div class="">Félix</div><div class=""><br class=""></div></body></html>