Quantcast

File and directory primitives are severely broken

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

File and directory primitives are severely broken

Ian Piumarta
Many of the file primitives allocate 1000-byte buffers for path names  
and pass their address to functions defined in support code, without  
passing their length or in any way trying to agree with support code  
about what the maximum length of a path name should be.  Directory  
primitives are worse: in one place a 256-byte buffer is allocated on  
the stack and expected to be sufficient for any directory entry.

Here's the portable way to fix this without changing any of the APIs.

I suggest everyone who maintains a platform check that they do indeed  
have <limits.h> (its availability is mandated by ANSI C) and once  
nobody reports it missing that this header file be included at the  
beginning of sq.h.  All occurrences of

        char buf[1000];
or
        char entryName[256];

that allocate space for path names in the file and directory prims  
should then be changed to read

        char buf[PATH_MAX];

before one of these ticking time bombs explodes in someone's face.

FWIW: The Unix VM uses PATH_MAX religiously when checking for over-
length, or allocating space for, path names.  (This is the correct  
thing to do.)  On OS X PATH_MAX is 1024; on Linux it's 4096.

Cheers,
Ian

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: File and directory primitives are severely broken

timrowledge

On 24-Apr-06, at 3:06 PM, Ian Piumarta wrote:

> Many of the file primitives allocate 1000-byte buffers for path  
> names and pass their address to functions defined in support code,  
> without passing their length or in any way trying to agree with  
> support code about what the maximum length of a path name should be.

Yup, very tacky. RISC OS and win32 don't actually use any of this  
code (Cross/plugins/filePlugin/sqFilePluginBasicPrims.c) though and  
so aren't affected in quite the same manner.

> Directory primitives are worse: in one place a 256-byte buffer is  
> allocated on the stack and expected to be sufficient for any  
> directory entry.
The implementation of primitiveDirectoryLookup in FilePlugin is  
certainly in error and it really should pass in the size of that  
entryName buffer. Which it could trivially do simply by setting the  
value of entryNameSize before calling dir_Lookup() without having to  
rely on limits.h. Should Slang code rely on a name defined in a .h  
file? Can't say I particularly care much either way.

>
> Here's the portable way to fix this without changing any of the APIs.
>
> I suggest everyone who maintains a platform check that they do  
> indeed have <limits.h> (its availability is mandated by ANSI C) and  
> once nobody reports it missing that this header file be included at  
> the beginning of sq.h.

Sounds workable

>   All occurrences of
>
> char buf[1000];
> or
> char entryName[256];
>
> that allocate space for path names in the file and directory prims  
> should then be changed to read
>
> char buf[PATH_MAX];
>
> before one of these ticking time bombs explodes in someone's face.

Feel free to edit and commit.
>
> FWIW: The Unix VM uses PATH_MAX religiously when checking for over-
> length, or allocating space for, path names.  (This is the correct  
> thing to do.)  On OS X PATH_MAX is 1024; on Linux it's 4096.
So does RISC OS and I'm pretty sure so does win32 (uncertainty only  
because I start defocussing if I look at windwos API code), so no  
need to worry there.

I can change the VMM method for primitiveDirectoryLookup to
a) use PATH_MAX
b) set the entryNameSize just in case

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
"How many Slavers does it take to change a lightbulb?" "Dunno. How  
susceptible are lightbulbs to telepathy?"


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: File and directory primitives are severely broken

johnmci
In reply to this post by Ian Piumarta
mmm as part of this issues, I had suggested to Stef that someone  
needed to sit down (any takers) and document all the basic VM api  
calls that rely on host operating system support. Plus of course  
explore what to do about unsafe calls that have buffer overflow  
concerns.

Given that he was willing to find someone to help write a set of  
SUnits that test these interface points.


On 24-Apr-06, at 3:06 PM, Ian Piumarta wrote:

> Many of the file primitives allocate 1000-byte buffers for path  
> names and pass their address to functions defined in support code,  
> without passing their length or in any way trying to agree with  
> support code about what the maximum length of a path name should  
> be.  Directory primitives are worse: in one place a 256-byte buffer  
> is allocated on the stack and expected to be sufficient for any  
> directory entry.
>
> Here's the portable way to fix this without changing any of the APIs.
>
> I suggest everyone who maintains a platform check that they do  
> indeed have <limits.h> (its availability is mandated by ANSI C) and  
> once nobody reports it missing that this header file be included at  
> the beginning of sq.h.  All occurrences of
>
> char buf[1000];
> or
> char entryName[256];
>
> that allocate space for path names in the file and directory prims  
> should then be changed to read
>
> char buf[PATH_MAX];
>
> before one of these ticking time bombs explodes in someone's face.
>
> FWIW: The Unix VM uses PATH_MAX religiously when checking for over-
> length, or allocating space for, path names.  (This is the correct  
> thing to do.)  On OS X PATH_MAX is 1024; on Linux it's 4096.
>
> Cheers,
> Ian
>

--
========================================================================
===
John M. McIntosh <[hidden email]> 1-800-477-2659
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
========================================================================
===

Loading...