Merge pull request #15516 from microsoft/51-2cppnon-constant-format-alter-not-const-source

C++: Change sources in `NonConstantFormat.ql`
This commit is contained in:
Mathias Vorreiter Pedersen
2024-02-17 00:01:58 +01:00
committed by GitHub
8 changed files with 243 additions and 153 deletions

View File

@@ -23,7 +23,7 @@ extern char *dcngettext (const char *__domainname, const char *__msgid1,
extern char *any_random_function(const char *);
#define NULL ((void*)0)
#define _(X) any_random_function((X))
#define _(X) gettext(X)
int main(int argc, char **argv) {
if(argc > 1)
@@ -40,10 +40,9 @@ int main(int argc, char **argv) {
printf(gettext("%d arguments\n"), argc-1); // GOOD
printf(any_random_function("%d arguments\n"), argc-1); // BAD
// Even though `_` is mapped to `some_random_function` above,
// the following call should not be flagged.
printf(_(any_random_function("%d arguments\n")),
argc-1); // GOOD
printf(_(any_random_function("%d arguments\n")), argc-1); // BAD
return 0;
}

View File

@@ -1,24 +1,21 @@
| NonConstantFormat.c:30:10:30:16 | access to array | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| NonConstantFormat.c:41:9:41:27 | call to any_random_function | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| NonConstantFormat.c:45:9:45:48 | call to gettext | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| nested.cpp:21:23:21:26 | fmt0 | The format string argument to snprintf should be constant to prevent security issues and other potential errors. |
| nested.cpp:79:32:79:38 | call to get_fmt | The format string argument to diagnostic should be constant to prevent security issues and other potential errors. |
| nested.cpp:87:18:87:20 | fmt | The format string argument to diagnostic should be constant to prevent security issues and other potential errors. |
| test.cpp:51:10:51:21 | call to make_message | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:57:12:57:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:60:12:60:21 | call to const_wash | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:61:12:61:26 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:62:12:62:17 | + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:63:12:63:18 | * ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:64:12:64:18 | & ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:65:12:65:39 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:67:10:67:35 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:70:12:70:20 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:76:12:76:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:82:12:82:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:88:12:88:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:93:12:93:18 | ++ ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:100:12:100:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:110:12:110:24 | new[] | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:115:12:115:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:130:20:130:26 | access to array | The format string argument to sprintf should be constant to prevent security issues and other potential errors. |
| test.cpp:157:12:157:15 | data | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:170:12:170:14 | res | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:195:31:195:33 | str | The format string argument to StringCchPrintfW should be constant to prevent security issues and other potential errors. |
| test.cpp:197:11:197:14 | wstr | The format string argument to wprintf should be constant to prevent security issues and other potential errors. |
| test.cpp:205:12:205:20 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:206:12:206:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:211:12:211:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:217:12:217:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:223:12:223:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:228:12:228:18 | ++ ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:235:12:235:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:242:12:242:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:247:12:247:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |

View File

@@ -18,7 +18,7 @@ extern "C" int snprintf ( char * s, int n, const char * format, ... );
struct A {
void do_print(const char *fmt0) {
char buf[32];
snprintf(buf, 32, fmt0); // GOOD [FALSE POSITIVE]
snprintf(buf, 32, fmt0); // BAD, all paths from unknown const char*, not assuming literal
}
};
@@ -34,12 +34,12 @@ struct C {
void do_some_printing(const char *fmt) {
b.do_printing(fmt);
}
const char *ext_fmt_str(void);
const char *ext_fmt_str(void); // NOTE: not assuming result is literal
};
void foo(void) {
C c;
c.do_some_printing(c.ext_fmt_str()); // BAD [NOT DETECTED]
c.do_some_printing(c.ext_fmt_str());
}
struct some_class {
@@ -76,12 +76,12 @@ void diagnostic(const char *fmt, ...)
}
void bar(void) {
diagnostic (some_instance->get_fmt()); // BAD
diagnostic (some_instance->get_fmt()); // BAD const char* but not assuming literal
}
namespace ns {
class blab {
class blab {
void out1(void) {
char *fmt = (char *)__builtin_alloca(10);
diagnostic(fmt); // BAD

View File

@@ -54,66 +54,66 @@ int main(int argc, char **argv) {
{
char hello[] = "hello, World\n";
hello[0] = 'H';
printf(hello); // BAD
printf(hello); // GOOD
printf(_(hello)); // GOOD
printf(gettext(hello)); // GOOD
printf(const_wash(hello)); // BAD
printf((hello + 1) + 1); // BAD
printf(+hello); // BAD
printf(*&hello); // BAD
printf(&*hello); // BAD
printf((char*)(void*)+(hello+1) + 1); // BAD
printf(const_wash(hello)); // GOOD
printf((hello + 1) + 1); // GOOD
printf(+hello); // GOOD
printf(*&hello); // GOOD
printf(&*hello); // GOOD
printf((char*)(void*)+(hello+1) + 1); // GOOD
}
printf(("Hello, World\n" + 1) + 1); // BAD
printf(("Hello, World\n" + 1) + 1); // GOOD
{
const char *hello = "Hello, World\n";
printf(hello + 1); // BAD
printf(hello + 1); // GOOD
printf(hello); // GOOD
}
{
const char *hello = "Hello, World\n";
hello += 1;
printf(hello); // BAD
printf(hello); // GOOD
}
{
// Same as above block but using "x = x + 1" syntax
const char *hello = "Hello, World\n";
hello = hello + 1;
printf(hello); // BAD
printf(hello); // GOOD
}
{
// Same as above block but using "x++" syntax
const char *hello = "Hello, World\n";
hello++;
printf(hello); // BAD
printf(hello); // GOOD
}
{
// Same as above block but using "++x" as subexpression
const char *hello = "Hello, World\n";
printf(++hello); // BAD
printf(++hello); // GOOD
}
{
// Same as above block but through a pointer
const char *hello = "Hello, World\n";
const char **p = &hello;
(*p)++;
printf(hello); // BAD
printf(hello); // GOOD
}
{
// Same as above block but through a C++ reference
const char *hello = "Hello, World\n";
const char *&p = hello;
p++;
printf(hello); // BAD [NOT DETECTED]
printf(hello); // GOOD
}
if (gettext_debug) {
printf(new char[100]); // BAD
printf(new char[100]); // BAD [FALSE NEGATIVE: uninitialized value not considered a source]
}
{
const char *hello = "Hello, World\n";
const char *const *p = &hello; // harmless reference to const pointer
printf(hello); // GOOD [FALSE POSITIVE]
hello++; // modification comes after use and so does no harm
const char *const *p = &hello;
printf(hello); // GOOD
hello++;
}
printf(argc > 2 ? "More than one\n" : _("Only one\n")); // GOOD
@@ -154,7 +154,7 @@ void print_ith_message() {
void fmt_via_strcpy(char *data) {
strcpy(data, "some string");
printf(data); // BAD
printf(data); // GOOD [FALSE POSITIVE: Due to inaccurate dataflow killers]
}
void fmt_with_assignment() {
@@ -163,3 +163,107 @@ void fmt_with_assignment() {
x = y = "a";
printf(y); // GOOD
}
void fmt_via_strcpy_bad(char *data) {
char res[100];
strcpy(res, data);
printf(res); // BAD
}
int wprintf(const wchar_t *format,...);
typedef wchar_t *STRSAFE_LPWSTR;
typedef const wchar_t *STRSAFE_LPCWSTR;
typedef unsigned int size_t;
void StringCchPrintfW(
STRSAFE_LPWSTR pszDest,
size_t cchDest,
STRSAFE_LPCWSTR pszFormat,
...
);
void wchar_t_test_good(){
wchar_t wstr[100];
StringCchPrintfW(wstr, 100, L"STRING"); // GOOD
wprintf(wstr); // GOOD
}
void wchar_t_test_bad(wchar_t* str){
wchar_t wstr[100];
StringCchPrintfW(wstr, 100, str); // BAD
wprintf(wstr); // BAD
}
char* get_string();
void pointer_arithmetic_test_on_bad_string(){
{
const char *hello = get_string();
printf(hello + 1); // BAD
printf(hello); // BAD
}
{
const char *hello = get_string();
hello += 1;
printf(hello); // BAD
}
{
// Same as above block but using "x = x + 1" syntax
const char *hello = get_string();
hello = hello + 1;
printf(hello); // BAD
}
{
// Same as above block but using "x++" syntax
const char *hello = get_string();
hello++;
printf(hello); // BAD
}
{
// Same as above block but using "++x" as subexpression
const char *hello = get_string();
printf(++hello); // BAD
}
{
// Same as above block but through a pointer
const char *hello = get_string();
const char **p = &hello;
(*p)++;
printf(hello); // BAD
}
{
// Same as above block but through a C++ reference
const char *hello = get_string();
const char *&p = hello;
p++;
printf(hello); // BAD
}
{
const char *hello = get_string();
const char *const *p = &hello;
printf(hello); // BAD
}
}
struct struct1 {
char *non_const;
const char* const_member = "TEST";
char * const_member2 = "TEST";
struct struct2{
char *nested_non_const;
const char* nested_const_member = "TEST";
char * nested_const_member2 = "TEST";
} nested;
};
int test_uncalled_func_with_struct_param(struct struct1 *s) {
printf(s->non_const); // BAD [FALSE NEGATIVE]
printf(s->const_member); // GOOD
printf(s->const_member2); // GOOD
printf(s->nested.nested_non_const); // BAD [FALSE NEGATIVE]
printf(s->nested.nested_const_member); // GOOD
printf(s->nested.nested_const_member2); // GOOD
}

View File

@@ -1,4 +1,3 @@
| consts.cpp:81:9:81:10 | c8 | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| consts.cpp:86:9:86:10 | v1 | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| consts.cpp:91:9:91:10 | v2 | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| consts.cpp:95:9:95:10 | v3 | The format string argument to printf should be constant to prevent security issues and other potential errors. |

View File

@@ -75,7 +75,7 @@ void a() {
// GOOD: constFuncToArray() always returns a value from gc1, which is always constant
printf(constFuncToArray(0));
// BAD: format string is not constant
// BAD: format string is not constant [NOT DETECTED]
char c8[10];
sprintf(c8, "%d", 1);
printf(c8);