zetzit / zz

πŸΊπŸ™ ZetZ a zymbolic verifier and tranzpiler to bare metal C
MIT License
1.6k stars 52 forks source link

external c functions incorrectly accepted as safe #22

Open bitwave opened 4 years ago

bitwave commented 4 years ago

I played a bit, and this compiles:

using <stdio.h>::{printf, scanf};

export fn main() -> int {
    char foo[2];

    scanf("%s", foo);
    printf("hello unsound %s\n", foo);
    return 0;
}

this should be unsound for len(stdin) > 2 or 1 (depends on NUL-byte or not) ASAN finds this at runtime, but it feels unsound.

bitwave commented 4 years ago

Another example:

using <stdio.h>::{printf, scanf};

export fn main() -> int {
    char *foo = 0x1337;

    printf("hello unsound %s\n", foo);
    return 0;
}

This is even simpler and simply unsafe.

aep commented 4 years ago

thank you for the report. calls to libc functions cannot be proven, because they have no zz signatures. therefor the checker is just disabled on any expression containing an external C type

still unsure if

  1. this is fine
  2. or require to be wrapped in an unsafe {} block
  3. or be done like typescript where you can specify additional signatures for plain old C code.
bitwave commented 4 years ago

If you focusing on safety, everything which is not proven or assumed explicitely, should be unsafe.

peirick commented 4 years ago

i would go with an explicit unsafe block.

fogti commented 4 years ago

It should at least emit a warning, especially for known-unsafe c functions (e.g. like scanf on strings, gets, etc.).

References:

inherently vulnerable/unsafe C functions:

aep commented 4 years ago

this is currently blocked because an unsafe block is a scope, so this is not possible:


unsafe {
  int i = some_c_function();
}
int x = i;

and initialization in unsafe breaks SMT.

int i;
unsafe {
   i = some_c_function();
}
int x = i; // uninitialized read access

i'll either need to make unsafe an expression instead of a scope

int i = unsafe {
   some_c_function()
};
int x = i;

which means you need to wrap every call in unsafe rather than an entire list of statements.

or make it a block but ignore scoping so this is valid but harder to understand:

unsafe {
  int i = some_c_function();
}
int x = i;

another option is forcing c calls through a macro

int i = @extern(some_c_function());
int x = i;

the macro can do additional checks in first class library code rather than in the compiler

fogti commented 4 years ago

Could it be possible to make unsafe an expression, and for the less often needed block-unsafe add an unsafe_block, which creates an own scope?

aep commented 4 years ago

unsafe is now available in expression position:

    int e = unsafe<int>(errno);