1
0
Fork 0
mirror of https://github.com/eclipse-cdt/cdt synced 2025-07-23 17:05:26 +02:00

Bug 320187 - A codan checker for detecting format string vulnerabilities

This commit is contained in:
Alena Laskavaia 2011-02-08 02:51:59 +00:00
parent 42fa511831
commit 1e1fdab079
9 changed files with 744 additions and 1 deletions

View file

@ -45,6 +45,10 @@ problem.name.UnusedReturnValue = Unused return value
problem.description.NoReturn = No return statement in a function which is declared to return value
problem.messagePattern.NoReturn = No return, in function returning non-void
problem.name.NoReturn = No return
checker.name.FormatString = Format String
problem.description.FormatString = Finds statements lead to format string vulnerability (e.g. 'char[5] str; scanf("%10s", str);'
problem.messagePattern.FormatString = Format string vulnerability in ''{0}''
problem.name.FormatString = Format String Vulnerability
checker.name.AssignmentToItself = Assignment to itself
problem.messagePattern.AssignmentToItself = Assignment to itself ''{0}''
problem.name.AssignmentToItself = Assignment to itself

View file

@ -320,5 +320,19 @@
name="%checker.name.CaseBreak">
</problem>
</checker>
<checker
class="org.eclipse.cdt.codan.internal.checkers.fs.ScanfFormatStringSecurityChecker"
id="org.eclipse.cdt.codan.internal.checkers.ScanfFormatStringSecurityChecker"
name="%checker.name.FormatString">
<problem
category="org.eclipse.cdt.codan.core.categories.Security"
defaultEnabled="false"
defaultSeverity="Warning"
description="%problem.description.FormatString"
id="org.eclipse.cdt.codan.internal.checkers.ScanfFormatStringSecurityProblem"
messagePattern="%problem.messagePattern.FormatString"
name="%problem.name.FormatString">
</problem>
</checker>
</extension>
</plugin>

View file

@ -0,0 +1,177 @@
/*******************************************************************************
* Copyright (c) 2010 Meisam Fathi and others
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* Meisam Fathi - initial API and implementation
*******************************************************************************/
package org.eclipse.cdt.codan.internal.checkers.fs;
import java.util.Collection;
import java.util.Iterator;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
/**
* This class parses the format string argument and extracts all %s tokens.
* @version 0.2, June 04, 2010
* @author Meisam Fathi
*/
public class CFormatStringParser {
/**
* At least one digit should be present
*/
private static final String DIGIT_PATTERN = "[0-9][0-9]*";//$NON-NLS-1$
/**
* The general format for a <strong>format string</strong> argument is
* "%[*][size][modifier]type", in which type is one of the following items:
* <ul>
* <li>c: Single character.
* <li>d: Decimal integer.
* <li>e,E,f,g,G: Floating point.
* <li>o Octal: integer.
* <li>s: String of characters.
* <li>u: Unsigned decimal integer.
* <li>x,X: Hexadecimal integer.
* </ul>
*
* @see {@link http://www.cplusplus.com/reference/clibrary/cstdio/scanf/}
* for more information.
*/
private static final String STRING_FORMAT_PATTERN = "%[\\*]?[0-9]*[hlL]?[cdeEfgGsuxX]";//$NON-NLS-1$
/**
* If there is an asterisk in the format argument, then it cannot be
* vulnerable. If there is a [modifier] (i.e. hlL), then compiler warns.
* Hence, the only vulnerable arguments are arguments in which either there
* is no specified size, or there is a size greater than the size of the
* string.
*
* @see #FORMAT_STRING_PATTERN
*/
private static final String VULNERABLE_PATTERN = "%[0-9]*s";//$NON-NLS-1$
/**
* The pattern which represents a string format.
*/
private final Pattern argumentPattern;
/**
* The matcher which matches string format arguments.
*/
private final Matcher argumentMatcher;
/**
* The pattern which may lead to vulnerability in <code>scanf</code>
* function calls.
*/
private final Pattern vulnerablePattern;
/**
* I guess, this must be a concurrent Collection, but I'm not sure. --
* Meisam
*/
private final Collection<VulnerableFormatStringArgument> vulnerableArguments;
public final static int ARGUMENT_SIZE_NOT_SPECIFIED = -1;
/**
* Constructs an argument parser for the given argument.
*
* @param argument
*/
protected CFormatStringParser(final String argument) {
this.argumentPattern = Pattern.compile(STRING_FORMAT_PATTERN);
this.argumentMatcher = this.argumentPattern.matcher(argument);
this.vulnerablePattern = Pattern.compile(VULNERABLE_PATTERN);
this.vulnerableArguments = new ConcurrentLinkedQueue<VulnerableFormatStringArgument>();
extractVulnerableArguments();
}
/**
* If the given argument to this class is vulnerable, it returns true, else
* it return false.
*
* @return true if the format string argument is vulnerable.
*/
public boolean isVulnerable() {
return !this.vulnerableArguments.isEmpty();
}
public Iterator<VulnerableFormatStringArgument> getVulnerableArgumentsIterator() {
return this.vulnerableArguments.iterator();
}
/**
* This method is guaranteed to be invoked in the constructor of the class.
* DON'T invoke it yourself. It should be invoke only once.
*/
private void extractVulnerableArguments() {
/*
* I'm not sure if clearing the collection is necessary. -- Meisam Fathi
*/
this.vulnerableArguments.clear();
boolean hasMore = this.argumentMatcher.find();
int indexOfCurrentArgument = 0;
while (hasMore) {
final String formatString = this.argumentMatcher.group();
final String matchedArgument = formatString;
final Matcher vulnerabilityMatcher = this.vulnerablePattern
.matcher(matchedArgument);
final boolean isVulnerable = vulnerabilityMatcher.find();
if (isVulnerable) {
final int argumentSize = parseArgumentSize(formatString);
final VulnerableFormatStringArgument vulnerableArgument = new VulnerableFormatStringArgument(
indexOfCurrentArgument, formatString, argumentSize);
this.vulnerableArguments.add(vulnerableArgument);
}
hasMore = this.argumentMatcher.find();
indexOfCurrentArgument++;
}
}
/**
* This method takes a string as input. The format of the input string is
* %[0-9]*s. If there is no digit present in the given string it returns
* <code>ARGUMENT_SIZE_NOT_SPECIFIED</code>, otherwise it returns the number specified after "%". For example:
* <ul>
* <li>%s ==> -1</li>
* <li>%123s ==> 123</li>
* <li>%1s ==> 1</li>
* <li>%015s ==> 15</li>
* <li>%0s ==> 0</li>
* </ul>
*
* @param formatString
* The given format string.
* @return Either ARGUMENT_SIZE_NOT_SPECIFIED or the number embedded in the input string.
*/
private int parseArgumentSize(final String formatString) {
// The minimum possible size for a string of format %[0-9]*s
final int MINIMUM_POSSIBLE_SIZE = 2;
int argumentSize = ARGUMENT_SIZE_NOT_SPECIFIED;
if (formatString.length() > MINIMUM_POSSIBLE_SIZE) {
final Pattern numberPattern = Pattern.compile(DIGIT_PATTERN);
final Matcher numberMatcher = numberPattern.matcher(formatString);
if (numberMatcher.find()) {
final String sizeModifierString = numberMatcher.group();
argumentSize = Integer.parseInt(sizeModifierString);
}
}
return argumentSize;
}
}

View file

@ -0,0 +1,201 @@
/*******************************************************************************
* Copyright (c) 2010 Meisam Fathi and others
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* Meisam Fathi - initial API and implementation
*******************************************************************************/
package org.eclipse.cdt.codan.internal.checkers.fs;
import java.util.Iterator;
import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker;
import org.eclipse.cdt.core.dom.ast.ASTVisitor;
import org.eclipse.cdt.core.dom.ast.IASTExpression;
import org.eclipse.cdt.core.dom.ast.IASTFunctionCallExpression;
import org.eclipse.cdt.core.dom.ast.IASTIdExpression;
import org.eclipse.cdt.core.dom.ast.IASTInitializerClause;
import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
import org.eclipse.cdt.core.dom.ast.IArrayType;
import org.eclipse.cdt.core.dom.ast.IType;
/**
* This checker detects format string vulnerabilities in the source code of
* C/C++ applications.
* <p>
* e.g:
* <p>
* <code>
* int f() { <br>
* char inputstr[5]; <br>
* scanf("%s", inputstr); // detects vulnerability here <br>
* return 0; <br>
* }
* </code>
* <p>
* e.g:
* <p>
* <code>
* int f(void) { <br>
* char inputstr[5]; <br>
* int inputval; <br>
* int i = 5; <br>
* scanf("%d %9s", inputval, inputstr); // detects vulnerability here <br>
* printf("%d" ,i); <br>
* return 0; <br>
* } <br>
* </code> <br>
* <p>
* e.g:
* <p>
* <code>
* int main(void) { <br>
* char inputstr[5]; <br>
* int inputval; <br>
* int i = 5; <br>
* scanf("%4s %i", inputstr, inputval); // no vulnerability here <br>
* printf("%d" ,i); <br>
* return 0; <br>
* } <br>
* </code>
*
* @version 0.3 July 29, 2010
* @author Meisam Fathi
*
*/
public class ScanfFormatStringSecurityChecker extends AbstractIndexAstChecker {
private static final String ER_ID = "org.eclipse.cdt.codan.internal.checkers.ScanfFormatStringSecurityProblem"; //$NON-NLS-1$
private final static VulnerableFunction[] VULNERABLE_FUNCTIONS = {//
// list of all format string vulnerable functions
new VulnerableFunction("scanf", 0), //$NON-NLS-1$
new VulnerableFunction("fscanf", 1), //$NON-NLS-1$
new VulnerableFunction("fwscanf", 1), //$NON-NLS-1$
new VulnerableFunction("wscanf", 0), //$NON-NLS-1$
new VulnerableFunction("swscanf", 1), //$NON-NLS-1$
new VulnerableFunction("sscanf", 1) //$NON-NLS-1$
};
public void processAst(IASTTranslationUnit ast) {
ast.accept(new FormatStringVisitor());
}
private static final class VulnerableFunction {
private final String name;
private final int formatStringArgumentIndex;
private VulnerableFunction(String name, int formatStringArgumentIndex) {
this.name = name;
this.formatStringArgumentIndex = formatStringArgumentIndex;
}
/**
* @return the name
*/
public String getName() {
return name;
}
/**
* @return the formatStringArgumentIndex
*/
public int getFormatStringArgumentIndex() {
return formatStringArgumentIndex;
}
}
private class FormatStringVisitor extends ASTVisitor {
private FormatStringVisitor() {
shouldVisitExpressions = true;
}
public int visit(IASTExpression expression) {
if (expression instanceof IASTFunctionCallExpression) {
IASTFunctionCallExpression callExpression = (IASTFunctionCallExpression) expression;
VulnerableFunction vulnerableFunction = getVulnerableFunctionForExpression(callExpression);
if (vulnerableFunction == null) {
return PROCESS_CONTINUE;
}
IASTInitializerClause[] arguments = callExpression
.getArguments();
int stringArgumentIndex = vulnerableFunction
.getFormatStringArgumentIndex();
detectFaulyArguments(callExpression, arguments,
stringArgumentIndex);
}
return PROCESS_CONTINUE;
}
private VulnerableFunction getVulnerableFunctionForExpression(
IASTFunctionCallExpression callExpression) {
String rawSignature = callExpression.getFunctionNameExpression()
.getRawSignature();
for (int i = 0; i < VULNERABLE_FUNCTIONS.length; i++) {
if (VULNERABLE_FUNCTIONS[i].getName().equals(rawSignature)) {
return VULNERABLE_FUNCTIONS[i];
}
}
return null;
}
private void detectFaulyArguments(
IASTFunctionCallExpression callExpression,
IASTInitializerClause[] arguments, int formatStringArgumentIndex) {
final IASTInitializerClause formatArgument = arguments[formatStringArgumentIndex];
final String formatArgumentValue = formatArgument.getRawSignature();
final CFormatStringParser formatStringParser = new CFormatStringParser(
formatArgumentValue);
if (!formatStringParser.isVulnerable()) {
return;
}
// match arguments;
final Iterator<VulnerableFormatStringArgument> vulnerableArgumentsIterator = formatStringParser
.getVulnerableArgumentsIterator();
while (vulnerableArgumentsIterator.hasNext()) {
final VulnerableFormatStringArgument currentArgument = vulnerableArgumentsIterator
.next();
final int argumentIndex = currentArgument.getArgumentIndex();
final int argumentSize = currentArgument.getArgumentSize();
if (argumentSize == CFormatStringParser.ARGUMENT_SIZE_NOT_SPECIFIED) {
reportProblem(ER_ID, callExpression,
callExpression.getRawSignature());
}
// else there some size is specified, so it should be less than
// or equal
// the size of the string variable.
int suspectArgumentIndex = 1 + formatStringArgumentIndex
+ argumentIndex;
IASTInitializerClause suspectArgument = arguments[suspectArgumentIndex];
if (suspectArgument instanceof IASTIdExpression) {
final IASTIdExpression idExpression = (IASTIdExpression) suspectArgument;
IType expressionType = idExpression.getExpressionType();
if (expressionType instanceof IArrayType) {
IArrayType arrayExpressionType = (IArrayType) expressionType;
long arraySize = arrayExpressionType.getSize()
.numericalValue().longValue();
if (argumentSize > arraySize) {
reportProblem(ER_ID, idExpression,
idExpression.getRawSignature());
}
}
}
}
}
}
}

View file

@ -0,0 +1,71 @@
/*******************************************************************************
* Copyright (c) 2010 Meisam Fathi and others
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* Meisam Fathi - initial API and implementation
*******************************************************************************/
package org.eclipse.cdt.codan.internal.checkers.fs;
/**
* @version 0.2 February 16, 2010
* @author Meisam Fathi
*/
public class VulnerableFormatStringArgument {
/**
* The index of the argument that is matched, starting at zero.
*/
private final int indexOfArgument;
/**
* The string format argument that may contain the fault
*/
private final String argument;
/**
* the size of the argument.
* <ul>
* <li><code>%15s ==> 15 </code>
* <li><code>%128s ==> 128 </code>
* <li><code>%s ==> infinity </code>
* </ul>
*/
private final int size;
/**
* @param indexOfCurrentArgument
* @param group
*/
public VulnerableFormatStringArgument(final int indexOfArgument,
final String rgument, final int size) {
this.indexOfArgument = indexOfArgument;
this.argument = rgument;
this.size = size;
}
/**
* @return the indexOfArgument
*/
public int getArgumentIndex() {
return this.indexOfArgument;
}
/**
* @return the argument
*/
public String getArgument() {
return this.argument;
}
/**
* @return
*/
public int getArgumentSize() {
return this.size;
}
}

View file

@ -0,0 +1,267 @@
/*******************************************************************************
* Copyright (c) 2010 Meisam Fathi and others
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* Meisam Fathi - base API
*******************************************************************************/
package org.eclipse.cdt.codan.core.internal.checkers;
import org.eclipse.cdt.codan.core.test.CheckerTestCase;
/**
* Test for {@see FormatStringChecker} class
*
*/
public class FormatStringCheckerTest extends CheckerTestCase {
@Override
public void setUp() throws Exception {
super.setUp();
enableProblems("org.eclipse.cdt.codan.internal.checkers.ScanfFormatStringSecurityProblem"); //$NON-NLS-1$
}
// int f(){
// return 0;
// }
public void testBase() {
loadCodeAndRun(getAboveComment());
checkNoErrors();
}
// int f(){
// char inputstr[5];
// scanf("%s", inputstr); // here
// return 0;
// }
public void testSimple() {
loadCodeAndRun(getAboveComment());
checkErrorLine(3);
}
// int main(void) {
// char inputstr1[5];
// int inputval;
// int i = 5;
// scanf("%i %4s", inputval, inputstr1); // no error here
// printf("%d" ,i);
// return 0;
// }
public void testIntRight() {
loadCodeAndRun(getAboveComment());
checkNoErrors();
}
// int main(void) {
// char inputstr[5];
// int inputval;
// int i = 5;
// scanf("%d %9s", inputval, inputstr);
// printf("%d" ,i);
// return 0;
// }
public void testIntWrong() {
loadCodeAndRun(getAboveComment());
checkErrorLine(5);
}
// int main(void) {
// char inputstr1[5];
// int inputval;
// int i = 5;
// scanf("%4s %i", inputstr1, inputval);
// printf("%d" ,i);
// return 0;
// }
public void testRightInt() {
loadCodeAndRun(getAboveComment());
checkNoErrors();
}
// int main(void) {
// char inputstr1[5];
// char inputstr2[5];
// int i = 5;
// scanf("%4s %9s", inputstr1, inputstr2);
// printf("%d" ,i);
// return 0;
// }
public void testRightWrong() {
loadCodeAndRun(getAboveComment());
checkErrorLine(5);
}
// int main(void) {
// char inputstr1[5];
// int inputval;
// int i = 5;
// scanf("%9s %i", inputstr1, inputval);
// printf("%d" ,i);
// return 0;
// }
public void testWrongInt() {
loadCodeAndRun(getAboveComment());
checkErrorLine(5);
}
// int main(void) {
// char inputstr1[5];
// char inputstr2[5];
// int i = 5;
// scanf("%9s %4s", inputstr1, inputstr2);
// printf("%d" ,i);
// return 0;
// }
public void testWrongRight() {
loadCodeAndRun(getAboveComment());
checkErrorLine(5);
}
// int main(void) {
// char inputstr[5];
// int i = 5;
// scanf("%s", inputstr);
// printf("%d" ,i);
// return 0;
// }
public void testInfiniteSize() {
loadCodeAndRun(getAboveComment());
checkErrorLine(4);
}
// int main(void) {
// puts("Enter a string whose length is bellow 5:");
// char inputstr[5];
// int i = 5;
// scanf("%3s", inputstr);
// printf("%d" ,i);
// return 0;
// }
public void testRight() {
loadCodeAndRun(getAboveComment());
checkNoErrors();
}
// int main(void) {
// char inputstr1[5];
// char inputstr2[5];
// int i = 5;
// scanf("%3s %4s", inputstr1, inputstr2);
// printf("%d" ,i);
// return 0;
// }
public void testRightRight() {
loadCodeAndRun(getAboveComment());
checkNoErrors();
}
// int main(void) {
// char inputstr1[5];
// char inputstr2[5];
// int i = 5;
// scanf("%8s %9s", inputstr1, inputstr2);
// printf("%d" ,i);
// return 0;
// }
public void testWrongWrong() {
loadCodeAndRun(getAboveComment());
checkErrorLine(5);
}
// char inputstr[5];
// int foo(void){
// char inputstr[15];
// return 0;
// }
// int main(void) {
// puts("Enter a string whose length is bellow 5:");
// int i = 5;
// scanf("%10s", inputstr);
// printf("%d" ,i);
// return 0;
// }
public void testGlobalBeforeWrong() {
loadCodeAndRun(getAboveComment());
checkErrorLine(9);
}
// int main(void) {
// puts("Enter a string whose length is bellow 5:");
// char inputstr[15];
// int i = 5;
// scanf("%10s", inputstr);
// printf("%d" ,i);
// return 0;
// }
// int foo(void){
// char inputstr[5];
// return 0;
// }
public void testNonglobalAfterRight() {
loadCodeAndRun(getAboveComment());
checkNoErrors();
}
// int main(void) {
// char inputstr[5];
// int i = 5;
// scanf("%10s", inputstr);
// printf("%d" ,i);
// return EXIT_SUCCESS;
// }
// int foo(void){
// char inputstr[15];
// return 0;
// }
public void testNonglobalAfterWrong() {
loadCodeAndRun(getAboveComment());
checkErrorLine(4);
}
// int foo(void){
// char inputstr[5];
// return 0;
// }
// int main(void) {
// char inputstr[15];
// int i = 5;
// scanf("%10s", inputstr);
// printf("%d" ,i);
// return 0;
// }
public void testNonglobalBeforeRight() {
loadCodeAndRun(getAboveComment());
checkNoErrors();
}
// int foo(void){
// char inputstr[15];
// return 0;
// }
// int main(void) {
// char inputstr[5];
// int i = 5;
// scanf("%10s", inputstr);
// printf("%d" ,i);
// return 0;
// }
public void testNonglobalBeforeWrong() {
loadCodeAndRun(getAboveComment());
checkErrorLine(8);
}
// int main(void) {
// char inputstr1[5];
// int inputval;
// int i = 5;
// scanf("%i %as", inputval, inputstr1); // no error here
// printf("%d" ,i);
// return 0;
// }
public void testGaurdedRight() {
loadCodeAndRun(getAboveComment());
checkNoErrors();
}
}

View file

@ -18,6 +18,7 @@ import org.eclipse.cdt.codan.core.internal.checkers.AssignmentInConditionChecker
import org.eclipse.cdt.codan.core.internal.checkers.AssignmentToItselfCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.CaseBreakCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.CatchByReferenceTest;
import org.eclipse.cdt.codan.core.internal.checkers.FormatStringCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.ReturnCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.ReturnStyleCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.StatementHasNoEffectCheckerTest;
@ -53,6 +54,7 @@ public class AutomatedIntegrationSuite extends TestSuite {
suite.addTestSuite(ReturnStyleCheckerTest.class);
suite.addTestSuite(SuspiciousSemicolonCheckerTest.class);
suite.addTestSuite(CaseBreakCheckerTest.class);
suite.addTestSuite(FormatStringCheckerTest.class);
// framework
suite.addTest(CodanFastTestSuite.suite());
// quick fixes

View file

@ -18,3 +18,6 @@ category.name.ProgrammingErrors = Potential Programming Problems
category.name.CodeStyle = Coding Style
category.name.CompilerErrors = Syntax and Semantic Errors
extension-point.name.CodeAnalysis = Code Analysis Checkers
marker.semanticError = Semantic Error
category.name.Security = Security Vulnerabilities

View file

@ -50,7 +50,7 @@
</extension>
<extension
id="codanSemanticProblem"
name="Semantic Error"
name="%marker.semanticError"
point="org.eclipse.core.resources.markers">
<super type="org.eclipse.cdt.codan.core.codanProblem"/>
@ -81,6 +81,10 @@
<category
id="org.eclipse.cdt.codan.core.categories.CompilerErrors"
name="%category.name.CompilerErrors"
/>
<category
id="org.eclipse.cdt.codan.core.categories.Security"
name="%category.name.Security"
/>
</extension>