Skip to content
This repository has been archived by the owner on Jun 17, 2022. It is now read-only.

external c functions incorrectly accepted as safe #22

Open
bitwave opened this issue Feb 11, 2020 · 8 comments
Open

external c functions incorrectly accepted as safe #22

bitwave opened this issue Feb 11, 2020 · 8 comments
Labels
inconsistent prover doesn't match C behaviour need-realworld-feedback feedback from users needed on how the decision would affect real world use
Milestone

Comments

@bitwave
Copy link

bitwave commented Feb 11, 2020

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
Copy link
Author

bitwave commented Feb 11, 2020

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
Copy link
Collaborator

aep commented Feb 11, 2020

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.

@aep aep added the need-realworld-feedback feedback from users needed on how the decision would affect real world use label Feb 11, 2020
@bitwave
Copy link
Author

bitwave commented Feb 12, 2020

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

@peirick
Copy link
Contributor

peirick commented Feb 12, 2020

i would go with an explicit unsafe block.

@fogti
Copy link

fogti commented Feb 13, 2020

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:

  • gets
  • getpw
  • rewind, setbuf
  • strcat, strcpy
  • sprintf

@aep aep changed the title Minimal unsound example external c functions incorrectly accepted as safe Feb 14, 2020
@aep aep added the inconsistent prover doesn't match C behaviour label Feb 14, 2020
@aep aep mentioned this issue Feb 14, 2020
4 tasks
@aep aep added this to the 0.1 milestone Mar 16, 2020
@aep
Copy link
Collaborator

aep commented Mar 30, 2020

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
Copy link

fogti commented Mar 30, 2020

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?

This was referenced Apr 4, 2020
@aep
Copy link
Collaborator

aep commented May 27, 2020

unsafe is now available in expression position:

    int e = unsafe<int>(errno);

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
inconsistent prover doesn't match C behaviour need-realworld-feedback feedback from users needed on how the decision would affect real world use
Projects
None yet
Development

No branches or pull requests

4 participants