Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support variadic function calls in checked scope #1174

Merged
merged 3 commits into from
Sep 2, 2021
Merged

Support variadic function calls in checked scope #1174

merged 3 commits into from
Sep 2, 2021

Conversation

mgrang
Copy link

@mgrang mgrang commented Aug 30, 2021

We add support for calling variadic functions in checked scope. These are
functions like printf, scanf, etc that take a format string and have a variable
number of arguments. We implement checking of arguments to these functions.
Following is a list of some important checks that we implement in checked scope
for these functions:

  • check that the argument corresponding to the %s format specifier is a
    null-terminated array.
  • all warnings emitted by the -Wformat family of flags have been converted to
    errors in checked scope.

We add support for calling variadic functions in checked scope. These are
functions like printf, scanf, etc that take a format string and have a variable
number of arguments. We implement checking of arguments to these functions.
Following is a list of some important checks that we implement in checked scope
for these functions:

- check that the argument corresponding to the %s format specifier is a
  null-terminated array.
- all warnings emitted by the -Wformat family of flags have been converted to
  errors in checked scope.
Copy link
Contributor

@sulekhark sulekhark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you!

Copy link
Contributor

@kkjeer kkjeer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

mgrang pushed a commit to checkedc/checkedc that referenced this pull request Sep 2, 2021
checkedc/checkedc-clang#1174 added support to call
variadic functions like printf/scanf, etc within checked scope. As a result,
tests that check for errors if such functions are invoked in checked scope
started failing. We fix one such test in this PR.
@mattmccutchen-cci
Copy link
Member

I realize I may have waited a little too late to post this, but I tested this PR and found several holes in the new checking. If you save the code below as printf-checking-bugs.c and then run clang -o printf-checking-bugs printf-checking-bugs.c and ./printf-checking-bugs percent_n, etc., you can see the SEGV. If you don't want to address the problems in this PR, I can copy the examples to a new issue.

#pragma CHECKED_SCOPE on

#include <string.h>
#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>

#define TEST(_name) else if (strcmp(test_name, #_name) == 0)

int main(int argc, _Nt_array_ptr<_Nt_array_ptr<char>> argv : count(argc)) {
  _Nt_array_ptr<char> test_name = argv[1];
  if (!test_name) {
    fprintf(stderr, "No test name specified\n");
    return 1;
  }

  if (0) {}
  TEST(percent_n) {
    // Missing check that %n argument is a _Ptr.
    int arr _Checked[1];
    printf("hello\n%n", arr + 123456789);
  }
  TEST(scanf_scalar) {
    // Missing check that _any_ scalar scanf argument is a _Ptr.
    int arr _Checked[1];
    sscanf("42", "%d", arr + 123456789);
  }
  TEST(printf_s_count) {
    // Missing check that printf %s argument has at least count(0).
    char buf _Nt_checked[1];
    printf("%s", buf + 123456789);
  }
  TEST(scanf_p) {
    // scanf reads an arbitrary _Ptr<void> via %p. The right solution here may
    // be to disallow _Ptr<void>
    // (https://github.com/microsoft/checkedc/issues/335). I couldn't find any
    // other way to exploit %p, but that doesn't mean there isn't any.
    _Ptr<void> q = 0;
    sscanf("0x1", "%p", &q);
    _Ptr<char> p = (_Ptr<char>)q;
    (*p)++;
  }
  TEST(scanf_s_overflow) {
    // scanf %s overflows the output buffer. I guess the compiler should require
    // the format string to specify a maximum width and check it against the
    // bounds of the argument?
    char field _Nt_checked[10];
    char input _Nt_checked[1000];
    memset(input, 'x', sizeof input - 1);
    sscanf(input, "%s", field);
  }
  else {
    fprintf(stderr, "Unknown test name\n");
    return 1;
  }

  return 0;
}

@mgrang
Copy link
Author

mgrang commented Sep 2, 2021

Thanks @mattmccutchen-cci for identifying these issues. I have filed #1178 to track these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants