Skip to content

Support variadic function calls in checked scope#1174

Merged
mgrang merged 3 commits into
masterfrom
varargs2
Sep 2, 2021
Merged

Support variadic function calls in checked scope#1174
mgrang merged 3 commits into
masterfrom
varargs2

Conversation

@mgrang

@mgrang mgrang commented Aug 30, 2021

Copy link
Copy Markdown

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.
@mgrang mgrang requested review from kkjeer and sulekhark August 30, 2021 19:58

@sulekhark sulekhark left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Thank you!

@kkjeer kkjeer left a comment

Copy link
Copy Markdown
Contributor

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

mgrang commented Sep 2, 2021

Copy link
Copy Markdown
Author

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