Security Issue VFS

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Security Issue VFS

Maarten-2

Hello,

Holger Fretyher and I concluded that there's a security issue in the
VFSAddOns package.

Code like this:

PackageLoader fileInPackage: 'VFSAddOns'.
((File name: 'dontcare') zip) createDirectory: '; xterm'.

Will not only try to open the zip, but also execute xterm, which
shouldn't be possible.
Now I'm wondering what would be the best way to fix this.

Paolo Bonzini suggested that doing something like:

st> 'abc'';xterm' asFile displayNl
'abc'\'';xterm'

might fix something.

I wonder if this would suffice or if there probably exists something
like the execvp system call for gnu-smalltalk?

Also VFSAddOns contained two bugs which made it impossible to use, I
think I've fixed those now so I'll try to submit those later. Where
should I do this?

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk
Reply | Threaded
Open this post in threaded view
|

Re: Security Issue VFS

Paolo Bonzini-2
On 11/16/2011 03:31 PM, maarten wrote:

>
> Hello,
>
> Holger Fretyher and I concluded that there's a security issue in the
> VFSAddOns package.
>
> Code like this:
>
> PackageLoader fileInPackage: 'VFSAddOns'.
> ((File name: 'dontcare') zip) createDirectory: '; xterm'.
>
> Will not only try to open the zip, but also execute xterm, which
> shouldn't be possible.
> Now I'm wondering what would be the best way to fix this.
>
> Paolo Bonzini suggested that doing something like:
>
> st> 'abc'';xterm' asFile displayNl
> 'abc'\'';xterm'
>
> might fix something.
>
> I wonder if this would suffice or if there probably exists something
> like the execvp system call for gnu-smalltalk?

It is on my todo list (and has been for a while) to write a class for
something like the posix_spawn API.  Ideally, that class would let you
attach arbitrary files/URLs/pipes to file descriptors in the child, and
then spawn the child.  Such an interface would also let you choose
between a parsed and unparsed command line.

Another simpler possibility would be to add something like

     Smalltalk system: #('zip' 'abc' 'def')

... that would automatically escape each argument.  However this assumes
that you do not need any redirection or piping, because in that case the
'>' or '|' would be escaped too.

A third possibility hence is to have

     Smalltalk system: 'zip %1 %2 > %3'
          withArguments: {'abc'. 'def'. 'ghi'}

that would let the user choose what to escape and what not.

> Also VFSAddOns contained two bugs which made it impossible to use, I
> think I've fixed those now so I'll try to submit those later. Where
> should I do this?

Here is fine, or a pull request on github.

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk
Reply | Threaded
Open this post in threaded view
|

Re: Security Issue VFS

Maarten-2
On 11/16/2011 03:45 PM, Paolo Bonzini wrote:

> On 11/16/2011 03:31 PM, maarten wrote:
>>
>> Hello,
>>
>> Holger Fretyher and I concluded that there's a security issue in the
>> VFSAddOns package.
>>
>> Code like this:
>>
>> PackageLoader fileInPackage: 'VFSAddOns'.
>> ((File name: 'dontcare') zip) createDirectory: '; xterm'.
>>
>> Will not only try to open the zip, but also execute xterm, which
>> shouldn't be possible.
>> Now I'm wondering what would be the best way to fix this.
>>
>> Paolo Bonzini suggested that doing something like:
>>
>> st> 'abc'';xterm' asFile displayNl
>> 'abc'\'';xterm'
>>
>> might fix something.
>>
>> I wonder if this would suffice or if there probably exists something
>> like the execvp system call for gnu-smalltalk?
>
> It is on my todo list (and has been for a while) to write a class for
> something like the posix_spawn API. Ideally, that class would let you
> attach arbitrary files/URLs/pipes to file descriptors in the child, and
> then spawn the child. Such an interface would also let you choose
> between a parsed and unparsed command line.
>
> Another simpler possibility would be to add something like
>
> Smalltalk system: #('zip' 'abc' 'def')
>
> ... that would automatically escape each argument. However this assumes
> that you do not need any redirection or piping, because in that case the
> '>' or '|' would be escaped too.
>
> A third possibility hence is to have
>
> Smalltalk system: 'zip %1 %2 > %3'
> withArguments: {'abc'. 'def'. 'ghi'}
>
> that would let the user choose what to escape and what not.
>
>> Also VFSAddOns contained two bugs which made it impossible to use, I
>> think I've fixed those now so I'll try to submit those later. Where
>> should I do this?
>
> Here is fine, or a pull request on github.
>
> _______________________________________________
> help-smalltalk mailing list
> [hidden email]
> https://lists.gnu.org/mailman/listinfo/help-smalltalk

Ok I'm considering this approach now,
I've written this function (just escapes operators):

escape [
        | index newstr operators |
        index := 1.
        newstr := ''.
        operators := #($~ $% $^ $\ $| $& $> $< $= $! $;).


        [ index <= (self size asInteger) ] whileTrue: [
            (operators includes: (self at: index)) ifTrue: [
                newstr := newstr, '\'.
            ].
            newstr := newstr, ((self at: index) asString).
            index := index + 1.
        ].

and added it to String.st in the kernel folder.
Now withing every call of system in the VFS library I've added  (string)
escape. This way anyone could escape any string in any situation and it
also works for this particular problem.


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk
Reply | Threaded
Open this post in threaded view
|

Re: Security Issue VFS

Paolo Bonzini-2
On 12/19/2011 04:41 PM, maarten wrote:
>
> and added it to String.st in the kernel folder.
> Now withing every call of system in the VFS library I've added  (string)
> escape. This way anyone could escape any string in any situation and it
> also works for this particular problem.

Your code is a bit inefficient.  Never use the comma message.  Always
use streams instead.

Also, a partial fix (not escaping everything) is as ineffective as no fix.

I attach a patch that does this more efficiently and adds
#system:withArguments:.  Can you fix VFS using this new method?

Paolo

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

esc.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Security Issue VFS

Holger Freyther
On 12/19/2011 06:24 PM, Paolo Bonzini wrote:
> +            #($% $" $< $> $| $& $^ $ ) do: [ :each | t at: each codePoint put: 1 ].

what about $' ? I am not very good with shell

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk
Reply | Threaded
Open this post in threaded view
|

Re: Security Issue VFS

Paolo Bonzini-2
On 12/19/2011 06:35 PM, Holger Hans Peter Freyther wrote:
>> >  +            #($% $" $<  $>  $| $&  $^ $ ) do: [ :each | t at: each codePoint put: 1 ].
> what about $' ? I am not very good with shell

This is the Windows side.

Paolo

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk
Reply | Threaded
Open this post in threaded view
|

Re: Security Issue VFS

Paolo Bonzini-2
In reply to this post by Paolo Bonzini-2
On 12/19/2011 06:24 PM, Paolo Bonzini wrote:
> Can you fix VFS using this new method?

Since I hope I'll be able to release 3.2.5 in the Christmas holidays,
here is the fix I'm applying.

Paolo

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

maarten.patch (3K) Download Attachment