[CLUE-Tech] OT: C code security - what to look for?

[Matt Gushee]

On Sun, Aug 10, 2003 at 10:36:37PM -0600, Bruce Ediger wrote:
> 
> Fourth, I thought it was a good joke.  It really didn't fit in at all.

Oh, yeah, the humor thing. These days I barely remember what it is to
laugh. But now that you mention it, I guess it is kinda funny.

On Mon, Aug 11, 2003 at 09:27:49AM -0600, Dale K. Hawkins wrote:
> Matt Gushee <matt at gushee.net> writes:
> 
> > Example: http://havenrock.com/pub/gd4o/gd4o-1.0a2.tar.gz  ;-)
> >          (if you do look at this, 'gdstubs.c' and the Makefile
> >           will be the most informative)
> > Explanation: http://caml.inria.fr/ocaml/htmlman/manual032.html
> 
> I was looking your code and it does look pretty good (from a cursory
> overview).

Thanks. Note, however, that most of the work so far was done by someone
else.

However, I would point out a couple of issues I noticed if
> 
> Let me further preface this by stating that my familiarity with ocaml
> and the ocaml C interface library now consists of looking at the
> referenced web page for all of two minutes.
> 
> 1.  Unchecked allocations:
> 
> The first thing that sticks out is the results of the alloc_* calls
> are not checked before the result is used. 

Okay, I had a feeling there would be things like that. I'm not sure that
those calls are problematic, but I'm not sure they're not, either. Those
alloc_* functions are designed to allocate memory in the OCaml heap,
which means they are subject to garbage collection. So if they are being
used properly, they will probably not cause memory leaks, at least. But
I'm not sure if they're being used properly or not; I'll make a point of
looking them over carefully.

Also: I understand in principle the (possible) need to check the
results, but what about them would need to be check, exactly?

> 2.  Unchecked assumptions:
> 
> One of the techniques utilized by the truly anal, is to check every
> assumption going into and just before returning from a function call
> (check your invariants).  The C++ FAQs has a good section on this.

URL?

> Basically, never take for granted that which you can check.
> 
> Random example for your stubs:
> 
> > value ml_image_dline(value *argv, int argn) {
> >   return
> >     ml_image_line_native(argv[0], argv[1], argv[2], argv[3], argv[4],
> >                          argv[5], argv[6]);
> > }
> 
> What is argn?  Why is it not used?

Good questions. argn represents the number of arguments, and I'm not
sure why it isn't being used. Seems a little sloppy. I had already
noticed that, just hadn't gotten around to figuring out what to do about
it.

BTW, the reason for functions being written that way has to do with
OCaml having a byte compiler and a native code compiler. When you
compile to native code, you can call all the "normal" functions directly
(those called xyz_native in gdstubs.c). But due to some limitation of
the byte compiler, you can't call a C function with more than 5
arguments from OCaml byte code. So you need wrapper functions like the
above that batch all the args into an array.

> At the very least, I'd do the following:
> 
> > value ml_image_dline(value *argv, int argn) {
> > 
> >   assert(argn == 7 && "Wrong number of arguments given to ml_image_dline");

Sounds like a good idea. Is that assert function standard in ANSI C? Or
is it at least portable among "normal" systems (whatever that means).

> Some other points to consider:
> 
> 1.  Unchecked file access:
> 
> The binding seems to allow the ability to open files (unchecked) any
> in the filesystem.  I cannot offer you any practical advise other then
> an deep, heartfelt *shudder*!

Ah. Do you mean that one could circumvent file permissions with this
code? That would indeed be something to watch out for, I guess.

Maybe I should do something like

  #ifdef ALLOW_FILE_ACCESS

> sort of hole was used to crack a box.  The famous was a challenge
> which pitted a windoze box v. a linux box about four years ago.  The
> linux web server had a cgi script serving up advertisement banners
> with unchecked gif images and *whamo*.

There's that word again: 'unchecked'. It doesn't sound good, but I'm not
picturing exactly what sort of problems it entails.

> B.  Send an invalid image reference to a given function call (this
>     would require creating a way to verify an image handle is truly
>     valid before doing any operations).

Hmm ... I'm not sure if this is a problem or not. If someone could plant
a C executable that accessed the wrapper library, it might be. But if
somebody can plant arbitrary executables on my system, they can do just
about anything they like without ever touching that library, right?

As far as OCaml is concerned, the situation you describe is basically
impossible, due to OCaml's strong typing system. To take a simple
example, if I have a function with this signature

  val foo : int -> int   (* takes an int, returns an int *)

And I try to do this

  let bar x y z =
    let x' = foo x in
    ...


  let baz a b c =
    let a' = bar a in
    ...


  let stuff = baz 3.0 'a' false;;


Doesn't matter how indirectly I call the 'foo' function, the compiler
will reject it as a type error.

Now, I suppose it is possible that an image object in OCaml could refer
to an image that has been destroyed in the C code, but that's the only 
way I can think of that it could be invalid and not be caught by the
compiler.

> Well, I guess I'd better get back to writing my unchecked, buggy,
> insecure programs.  Hope this is useful.

Very. Thanks!

-- 
Matt Gushee                 When a nation follows the Way,
Englewood, Colorado, USA    Horses bear manure through
mgushee at havenrock.com           its fields;
http://www.havenrock.com/   When a nation ignores the Way,
                            Horses bear soldiers through
                                its streets.
                                
                            --Lao Tzu (Peter Merel, trans.)