On Thu, Dec 7, 2017 at 1:12 AM, monty <monty2 at programmer.net> wrote:

> 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:
>
> https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/
> 3f49cd2f9a4833ea8e7d43e958bd216784204183/platforms/Cross/
> plugins/FilePlugin/sqFilePluginBasicPrims.c#L395
>
> https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/
> 66133d2224585cff9a1342b1e36962a9adf12ab8/platforms/win32/
> plugins/FilePlugin/sqWin32FilePrims.c#L175
>
> 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.
>

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.


>
> *Sent:* Sunday, December 03, 2017 at 8:28 AM
> *From:* "Brenda Larcom" <asparagi at hhhh.org>
> *To:* "The general-purpose Squeak developers list" <squeak-dev at lists.
> squeakfoundation.org>
> *Subject:* Re: [squeak-dev] #newFileNamed: has curious and ancient flaw
>
> 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
>
>
>
>


-- 
_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20171208/8d8aa37c/attachment.html>