From dff21e02dbb952bf24dc60115bf337432802bfdd Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 30 Oct 2019 15:03:53 +0000 Subject: [PATCH] CPP: Fully support positional arguments. --- cpp/ql/src/semmle/code/cpp/commons/Printf.qll | 57 +++++++++++++++++-- .../WrongTypeFormatArguments.expected | 20 ++++--- .../Linux_signed_chars/printf1.h | 36 ++++++------ 3 files changed, 82 insertions(+), 31 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll index 678a32736ca..7919c8c83d0 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll @@ -134,8 +134,15 @@ class FormattingFunctionCall extends Expr { * Gets the argument corresponding to the nth conversion specifier. */ Expr getConversionArgument(int n) { - result = this - .getFormatArgument(this.getFormat().(FormatLiteral).getFormatArgumentIndexFor(n, 2)) + exists(FormatLiteral fl | + fl = this.getFormat() and + ( + result = this.getFormatArgument(fl.getParameterFieldPositional(n)) + or + result = this.getFormatArgument(fl.getFormatArgumentIndexFor(n, 2)) and + not exists(fl.getParameterFieldPositional(n)) + ) + ) } /** @@ -144,8 +151,15 @@ class FormattingFunctionCall extends Expr { * an explicit minimum field width). */ Expr getMinFieldWidthArgument(int n) { - result = this - .getFormatArgument(this.getFormat().(FormatLiteral).getFormatArgumentIndexFor(n, 0)) + exists(FormatLiteral fl | + fl = this.getFormat() and + ( + result = this.getFormatArgument(fl.getMinFieldWidthPositional(n)) + or + result = this.getFormatArgument(fl.getFormatArgumentIndexFor(n, 0)) and + not exists(fl.getMinFieldWidthPositional(n)) + ) + ) } /** @@ -154,8 +168,15 @@ class FormattingFunctionCall extends Expr { * precision). */ Expr getPrecisionArgument(int n) { - result = this - .getFormatArgument(this.getFormat().(FormatLiteral).getFormatArgumentIndexFor(n, 1)) + exists(FormatLiteral fl | + fl = this.getFormat() and + ( + result = this.getFormatArgument(fl.getPrecisionPositional(n)) + or + result = this.getFormatArgument(fl.getFormatArgumentIndexFor(n, 1)) and + not exists(fl.getPrecisionPositional(n)) + ) + ) } /** @@ -354,6 +375,14 @@ class FormatLiteral extends Literal { */ string getParameterField(int n) { this.parseConvSpec(n, _, result, _, _, _, _, _) } + /** + * Gets the parameter field of the nth conversion specifier (if it has one) as a + * zero-based number. + */ + int getParameterFieldPositional(int n) { + result = this.getParameterField(n).regexpCapture("([0-9]*)\\$", 1).toInt() - 1 + } + /** * Gets the flags of the nth conversion specifier. */ @@ -423,6 +452,14 @@ class FormatLiteral extends Literal { */ int getMinFieldWidth(int n) { result = this.getMinFieldWidthOpt(n).toInt() } + /** + * Gets the zero-based parameter number of the minimum field width of the nth + * conversion specifier, if it is implicit and uses a positional parameter. + */ + int getMinFieldWidthPositional(int n) { + result = this.getMinFieldWidthOpt(n).regexpCapture("\\*([0-9]*)\\$", 1).toInt() - 1 + } + /** * Gets the precision of the nth conversion specifier (empty string if none is given). */ @@ -453,6 +490,14 @@ class FormatLiteral extends Literal { else result = this.getPrecisionOpt(n).regexpCapture("\\.([0-9]*)", 1).toInt() } + /** + * Gets the zero-based parameter number of the precision of the nth conversion + * specifier, if it is implicit and uses a positional parameter. + */ + int getPrecisionPositional(int n) { + result = this.getPrecisionOpt(n).regexpCapture("\\.\\*([0-9]*)\\$", 1).toInt() - 1 + } + /** * Gets the length flag of the nth conversion specifier. */ diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected index 441b62b1c2d..c67057710bb 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected @@ -16,8 +16,8 @@ | printf1.h:114:18:114:18 | d | This argument should be of type 'long double' but is of type 'double' | | printf1.h:147:19:147:19 | i | This argument should be of type 'long long' but is of type 'int' | | printf1.h:148:19:148:20 | ui | This argument should be of type 'unsigned long long' but is of type 'unsigned int' | -| printf1.h:159:18:159:18 | i | This argument should be of type 'char *' but is of type 'int' | | printf1.h:160:18:160:18 | i | This argument should be of type 'char *' but is of type 'int' | +| printf1.h:161:21:161:21 | s | This argument should be of type 'int' but is of type 'char *' | | printf1.h:167:17:167:17 | i | This argument should be of type 'char *' but is of type 'int' | | printf1.h:168:18:168:18 | i | This argument should be of type 'char *' but is of type 'int' | | printf1.h:169:19:169:19 | i | This argument should be of type 'char *' but is of type 'int' | @@ -35,18 +35,24 @@ | printf1.h:192:19:192:19 | s | This argument should be of type 'int' but is of type 'char *' | | printf1.h:193:22:193:22 | s | This argument should be of type 'int' but is of type 'char *' | | printf1.h:194:25:194:25 | i | This argument should be of type 'char *' but is of type 'int' | -| printf1.h:213:28:213:28 | s | This argument should be of type 'int' but is of type 'char *' | +| printf1.h:198:24:198:24 | s | This argument should be of type 'int' but is of type 'char *' | +| printf1.h:199:21:199:21 | i | This argument should be of type 'char *' but is of type 'int' | +| printf1.h:202:26:202:26 | s | This argument should be of type 'int' but is of type 'char *' | +| printf1.h:203:23:203:23 | i | This argument should be of type 'char *' but is of type 'int' | +| printf1.h:206:25:206:25 | s | This argument should be of type 'int' but is of type 'char *' | +| printf1.h:207:22:207:22 | i | This argument should be of type 'char *' but is of type 'int' | +| printf1.h:210:26:210:26 | s | This argument should be of type 'int' but is of type 'char *' | +| printf1.h:211:23:211:23 | i | This argument should be of type 'char *' but is of type 'int' | | printf1.h:214:28:214:28 | s | This argument should be of type 'int' but is of type 'char *' | | printf1.h:215:28:215:28 | s | This argument should be of type 'int' but is of type 'char *' | -| printf1.h:216:28:216:28 | s | This argument should be of type 'int' but is of type 'char *' | +| printf1.h:216:25:216:25 | i | This argument should be of type 'char *' but is of type 'int' | | printf1.h:221:18:221:18 | s | This argument should be of type 'int' but is of type 'char *' | | printf1.h:222:20:222:20 | s | This argument should be of type 'int' but is of type 'char *' | -| printf1.h:233:22:233:22 | s | This argument should be of type 'int' but is of type 'char *' | -| printf1.h:233:25:233:25 | i | This argument should be of type 'char *' but is of type 'int' | -| printf1.h:234:22:234:22 | s | This argument should be of type 'int' but is of type 'char *' | +| printf1.h:225:23:225:23 | i | This argument should be of type 'char *' but is of type 'int' | +| printf1.h:228:24:228:24 | i | This argument should be of type 'char *' but is of type 'int' | +| printf1.h:231:25:231:25 | i | This argument should be of type 'char *' but is of type 'int' | | printf1.h:234:25:234:25 | i | This argument should be of type 'char *' but is of type 'int' | | printf1.h:235:22:235:22 | s | This argument should be of type 'int' but is of type 'char *' | -| printf1.h:235:25:235:25 | i | This argument should be of type 'char *' but is of type 'int' | | real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' | | real_world.h:62:22:62:23 | & ... | This argument should be of type 'short *' but is of type 'int *' | | real_world.h:63:22:63:24 | & ... | This argument should be of type 'short *' but is of type 'unsigned int *' | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h index 6713648267e..4517cfadf56 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h @@ -156,9 +156,9 @@ void complexFormatSymbols(int i, const char *s) { // positional arguments printf("%1$i", i, s); // GOOD - printf("%2$s", i, s); // GOOD [FALSE POSITIVE] + printf("%2$s", i, s); // GOOD printf("%1$s", i, s); // BAD - printf("%2$i", i, s); // BAD [NOT DETECTED] + printf("%2$i", i, s); // BAD // width / precision printf("%4i", i); // GOOD @@ -195,22 +195,22 @@ void complexFormatSymbols(int i, const char *s) // positional arguments mixed with variable width / precision printf("%2$*1$s", i, s); // GOOD - printf("%2$*2$s", i, s); // BAD [NOT DETECTED] - printf("%1$*1$s", i, s); // BAD [NOT DETECTED] + printf("%2$*2$s", i, s); // BAD + printf("%1$*1$s", i, s); // BAD printf("%2$*1$.4s", i, s); // GOOD - printf("%2$*2$.4s", i, s); // BAD [NOT DETECTED] - printf("%1$*1$.4s", i, s); // BAD [NOT DETECTED] + printf("%2$*2$.4s", i, s); // BAD + printf("%1$*1$.4s", i, s); // BAD printf("%2$.*1$s", i, s); // GOOD - printf("%2$.*2$s", i, s); // BAD [NOT DETECTED] - printf("%1$.*1$s", i, s); // BAD [NOT DETECTED] + printf("%2$.*2$s", i, s); // BAD + printf("%1$.*1$s", i, s); // BAD printf("%2$4.*1$s", i, s); // GOOD - printf("%2$4.*2$s", i, s); // BAD [NOT DETECTED] - printf("%1$4.*1$s", i, s); // BAD [NOT DETECTED] + printf("%2$4.*2$s", i, s); // BAD + printf("%1$4.*1$s", i, s); // BAD - printf("%2$*1$.*1$s", i, s); // GOOD [FALSE POSITIVE] + printf("%2$*1$.*1$s", i, s); // GOOD printf("%2$*2$.*1$s", i, s); // BAD printf("%2$*1$.*2$s", i, s); // BAD printf("%1$*1$.*1$s", i, s); // BAD @@ -222,15 +222,15 @@ void complexFormatSymbols(int i, const char *s) printf("%1$-4i", s); // BAD printf("%1$-4s", s, i); // GOOD - printf("%2$-4s", s, i); // BAD [NOT DETECTED] + printf("%2$-4s", s, i); // BAD printf("%1$-.4s", s, i); // GOOD - printf("%2$-.4s", s, i); // BAD [NOT DETECTED] + printf("%2$-.4s", s, i); // BAD printf("%1$-4.4s", s, i); // GOOD - printf("%2$-4.4s", s, i); // BAD [NOT DETECTED] - - printf("%1$-*2$s", s, i); // GOOD [FALSE POSITIVE x2] - printf("%2$-*2$s", s, i); // BAD [ADDITIONAL RESULT IS A FALSE POSITIVE] - printf("%1$-*1$s", s, i); // BAD [ADDITIONAL RESULT IS A FALSE POSITIVE] + printf("%2$-4.4s", s, i); // BAD + + printf("%1$-*2$s", s, i); // GOOD + printf("%2$-*2$s", s, i); // BAD + printf("%1$-*1$s", s, i); // BAD }