On Dec 2, 2017, at 11:26 PM, monty <monty2 at programmer.net> wrote:

>> Sent: Saturday, November 25, 2017 at 9:46 PM
>> From: "tim Rowledge" <tim at rowledge.org>
>> To: "The general-purpose Squeak developers list" <squeak-dev at lists.squeakfoundation.org>
>> Subject: [squeak-dev] #newFileNamed: has curious and ancient flaw
>> 
>> … 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? 
> 
> ... 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.

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 (https://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/avoid-race.html).  Start with section 7.11.1.1 if you like to skim.

If I reviewed StandardFileStream 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.

> 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,

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.

> 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.

From a security perspective, I like this solution even better.  Developer visibility into errors is almost always helpful.

— Brenda
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20171203/e5b037b5/attachment.html>