<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Dec 7, 2017 at 1:12 AM, monty <span dir="ltr"><<a href="mailto:monty2@programmer.net" target="_blank">monty2@programmer.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div style="font-family:Verdana;font-size:12.0px"><div>
<div>You're the only other person to show concern over this. I wrote sqFileOpenNew() because of this (and fixed some race conditions in sqFileOpen() in the process). Here're the implementations:</div>
<div> </div>
<div><a href="https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/3f49cd2f9a4833ea8e7d43e958bd216784204183/platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c#L395" target="_blank">https://github.com/<wbr>OpenSmalltalk/opensmalltalk-<wbr>vm/blob/<wbr>3f49cd2f9a4833ea8e7d43e958bd21<wbr>6784204183/platforms/Cross/<wbr>plugins/FilePlugin/<wbr>sqFilePluginBasicPrims.c#L395</a></div>
<div> </div>
<div><a href="https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/66133d2224585cff9a1342b1e36962a9adf12ab8/platforms/win32/plugins/FilePlugin/sqWin32FilePrims.c#L175" target="_blank">https://github.com/<wbr>OpenSmalltalk/opensmalltalk-<wbr>vm/blob/<wbr>66133d2224585cff9a1342b1e36962<wbr>a9adf12ab8/platforms/win32/<wbr>plugins/FilePlugin/<wbr>sqWin32FilePrims.c#L175</a></div>
<div>
<div> </div>
<div>But I'd like to get the error codes working first, so we only need to support one version of the primitive in the image, and fallback on the broken Squeak code for older VMs that lack it.</div></div></div></div></div></blockquote><div><br></div><div>I'm wrestling with git as I type. The support for error codes is in the VMMaker source. It'll be in the git repository soon, and in the Squeak base image soon too. I have meetings planned for this morning but should have it done before Monday.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div style="font-family:Verdana;font-size:12.0px"><div><div>
<div> </div>
<div style="margin:10.0px 5.0px 5.0px 10.0px;padding:10.0px 0 10.0px 10.0px;border-left:2.0px solid rgb(195,217,229)">
<div style="margin:0 0 10.0px 0"><b>Sent:</b> Sunday, December 03, 2017 at 8:28 AM<br>
<b>From:</b> "Brenda Larcom" <<a href="mailto:asparagi@hhhh.org" target="_blank">asparagi@hhhh.org</a>><span class=""><br>
<b>To:</b> "The general-purpose Squeak developers list" <<a href="mailto:squeak-dev@lists.squeakfoundation.org" target="_blank">squeak-dev@lists.<wbr>squeakfoundation.org</a>><br>
</span><b>Subject:</b> Re: [squeak-dev] #newFileNamed: has curious and ancient flaw</div><div><div class="h5">
<div>
<div>
<div> </div>
<div>On Dec 2, 2017, at 11:26 PM, monty <<a href="mailto:monty2@programmer.net" target="_blank">monty2@programmer.net</a>> wrote:</div>
<div> </div>
<blockquote>
<div>
<blockquote><span>Sent: Saturday, November 25, 2017 at 9:46 PM</span></blockquote>
<blockquote><span>From: "tim Rowledge" <<a href="mailto:tim@rowledge.org" target="_blank">tim@rowledge.org</a>></span></blockquote>
<blockquote><span>To: "The general-purpose Squeak developers list" <<a href="mailto:squeak-dev@lists.squeakfoundation.org" target="_blank">squeak-dev@lists.<wbr>squeakfoundation.org</a>></span></blockquote>
<blockquote><span>Subject: [squeak-dev] #newFileNamed: has curious and ancient flaw</span></blockquote>
<blockquote> </blockquote>
<blockquote><span>… and another thing - take a look at StandardFileStream class>>#newFileNamed:. Note that it carefully checks to see if a file of the requested name already exists? </span></blockquote>
<br>
<span>... Checking to see if a file exists first and then opening it for writing/creation after is an obvious race condition, which could result in data loss. The open and existence check need to be done together atomically, failing when the file already exists. This is the type of little thing where if we don't get it right, we look amateur.</span></div>
</blockquote>
<div> </div>
This race condition in particular can result in exploitable security issues as well as accidental data loss issues. One classic reference on the topic (since at least the mid-1990s when I got into secure development professionally) is David Wheeler’s Secure Programming HOWTO (<a href="https://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/avoid-race.html" target="_blank">https://www.dwheeler.com/<wbr>secure-programs/Secure-<wbr>Programs-HOWTO/avoid-race.html</a><wbr>). Start with section 7.11.1.1 if you like to skim.
<div> </div>
<div>If I reviewed <span>StandardFileStream</span> <wbr>for a client, I would expect this issue to get at least a high severity. Based on what Tim & Monty said above & below (it may be yet more months before I can check an image or VM myself), using this method without creating security holes would be quite challenging.
<div>
<div>
<blockquote>
<div><span>We have sqFileOpenNew() in newer VMs (it's unused in the image though), but it uses a boolean flag parameter to distinguish between failure caused by the file existing and all other causes, </span></div>
</blockquote>
<div> </div>
This sounds like an incomplete past attempt to fix this (or a related) security hole. Starting to use this primitive instead could be a good short-term fix.<br>
<blockquote>
<div><span>and I'd rather have a set of FilePlugin error codes, which could be mapped to file exceptions in the image, to provide better info for why a file operation failed, and also to fix certain file methods I've seen that just assume a particular reason for a failure.</span></div>
</blockquote>
<div> </div>
>From a security perspective, I like this solution even better. Developer visibility into errors is almost always helpful.</div>
<div><br>
— Brenda</div>
</div>
</div>
</div>
</div>
</div></div></div>
</div>
</div></div></div>
<br><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><span style="font-size:small;border-collapse:separate"><div>_,,,^..^,,,_<br></div><div>best, Eliot</div></span></div></div></div>
</div></div>