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.

Sign up for GitHub

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

Jump to bottom

Fix #1516 - protect ExpressionEvaluator from self-referential macros #1517

Merged
merged 1 commit into from
Jul 18, 2018
Merged

Fix #1516 - protect ExpressionEvaluator from self-referential macros #1517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions cxx-squid/src/main/java/org/sonar/cxx/preprocessor/CxxPreprocessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ public boolean checkArgumentsCount(int count) {
private SourceCodeProvider codeProvider = new SourceCodeProvider();
private SourceCodeProvider unitCodeProvider;
private SquidAstVisitorContext<Grammar> context;
private ExpressionEvaluator ifExprEvaluator;
private List<String> cFilesPatterns;
private CxxConfiguration conf;
private CxxCompilationUnitSettings compilationUnitSettings;
Expand Down Expand Up @@ -248,7 +247,6 @@ public CxxPreprocessor(SquidAstVisitorContext<Grammar> context,
SourceCodeProvider sourceCodeProvider,
CxxLanguage language) {
this.context = context;
this.ifExprEvaluator = new ExpressionEvaluator(conf, this);
this.cFilesPatterns = conf.getCFilesPatterns();
this.conf = conf;
this.language = language;
Expand Down Expand Up @@ -518,8 +516,8 @@ PreprocessorAction handleIfLine(AstNode ast, Token token, String filename) { //T
}
try {
currentFileState.skipPreprocessorDirectives = false;
currentFileState.skipPreprocessorDirectives = !ifExprEvaluator.eval(
ast.getFirstDescendant(CppGrammar.constantExpression));
currentFileState.skipPreprocessorDirectives = !ExpressionEvaluator.eval(conf, this,
ast.getFirstDescendant(CppGrammar.constantExpression));
} catch (EvaluationException e) {
LOG.error("[{}:{}]: error evaluating the expression {} assume 'true' ...",
filename, token.getLine(), token.getValue());
Expand Down Expand Up @@ -553,8 +551,8 @@ PreprocessorAction handleElIfLine(AstNode ast, Token token, String filename) { /
filename, token.getLine(), token.getValue());
}
currentFileState.skipPreprocessorDirectives = false;
currentFileState.skipPreprocessorDirectives = !ifExprEvaluator.eval(
ast.getFirstDescendant(CppGrammar.constantExpression));
currentFileState.skipPreprocessorDirectives = !ExpressionEvaluator.eval(conf, this,
ast.getFirstDescendant(CppGrammar.constantExpression));
} catch (EvaluationException e) {
LOG.error("[{}:{}]: error evaluating the expression {} assume 'true' ...",
filename, token.getLine(), token.getValue());
Expand Down Expand Up @@ -658,7 +656,7 @@ private void parseIncludeLine(String includeLine, String filename, Charset chars
.getToken(), filename, charset);
}

PreprocessorAction handleIncludeLine(AstNode ast, Token token, String filename, Charset charset) { //TODO: deprecated
PreprocessorAction handleIncludeLine(AstNode ast, Token token, String filename, Charset charset) { //TODO: deprecated
//
// Included files have to be scanned with the (only) goal of gathering macros.
// This is done as follows:
Expand Down Expand Up @@ -1035,7 +1033,7 @@ private List<Token> replaceParams(List<Token> body, List<Token> parameters, List
}
}

// replace # with "" if sequence HASH BR occurs for body HASH __VA_ARGS__
// replace # with "" if sequence HASH BR occurs for body HASH __VA_ARGS__
if (newTokens.size() > 3 && newTokens.get(newTokens.size() - 2).getType().equals(HASH)
&& newTokens.get(newTokens.size() - 1).getType().equals(BR_RIGHT)) {
for (int n = newTokens.size() - 2; n != 0; n--) {
Expand Down
12 changes: 3 additions & 9 deletions cxx-squid/src/main/java/org/sonar/cxx/preprocessor/EvaluationException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,9 @@
package org.sonar.cxx.preprocessor;

public class EvaluationException extends RuntimeException {
private static final long serialVersionUID = 336015352128912495L;

private final String why;

public EvaluationException(String why) {
this.why = why;
}

@Override
public String toString() {
return why;
public EvaluationException(String message) {
super(message);
}
}
65 changes: 50 additions & 15 deletions cxx-squid/src/main/java/org/sonar/cxx/preprocessor/ExpressionEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,12 @@
import com.sonar.sslr.api.Token;
import com.sonar.sslr.impl.Parser;
import java.math.BigInteger;
import java.util.Deque;
import java.util.LinkedList;
import java.util.List;

import javax.annotation.Nullable;

import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.cxx.CxxConfiguration;
Expand All @@ -41,19 +45,21 @@ public final class ExpressionEvaluator {

private final Parser<Grammar> parser;
private final CxxPreprocessor preprocessor;
private final Deque<String> macroEvaluationStack;

public ExpressionEvaluator(CxxConfiguration conf, CxxPreprocessor preprocessor) {
private ExpressionEvaluator(CxxConfiguration conf, CxxPreprocessor preprocessor) {
parser = CppParser.createConstantExpressionParser(conf);

this.preprocessor = preprocessor;
macroEvaluationStack = new LinkedList<>();
}

public boolean eval(String constExpr) {
return evalToInt(constExpr, null).compareTo(BigInteger.ZERO) != 0;
public static boolean eval(CxxConfiguration conf, CxxPreprocessor preprocessor, String constExpr) {
return new ExpressionEvaluator(conf, preprocessor).evalToBoolean(constExpr, null);
}

public boolean eval(AstNode constExpr) {
return evalToInt(constExpr).compareTo(BigInteger.ZERO) != 0;
public static boolean eval(CxxConfiguration conf, CxxPreprocessor preprocessor, AstNode constExpr) {
return new ExpressionEvaluator(conf, preprocessor).evalToBoolean(constExpr);
}

private BigInteger evalToInt(String constExpr, @Nullable AstNode exprAst) {
Expand Down Expand Up @@ -86,6 +92,16 @@ private BigInteger evalToInt(AstNode exprAst) {
return evalComplexAst(exprAst);
}

private boolean evalToBoolean(AstNode exprAst)
{
return !BigInteger.ZERO.equals(evalToInt(exprAst));
}

private boolean evalToBoolean(String constExpr, @Nullable AstNode exprAst)
{
return !BigInteger.ZERO.equals(evalToInt(constExpr, exprAst));
}

private BigInteger evalLeaf(AstNode exprAst) {
// Evaluation of leafs
//
Expand All @@ -96,8 +112,27 @@ private BigInteger evalLeaf(AstNode exprAst) {
} else if (nodeType.equals(CxxTokenType.CHARACTER)) {
return evalCharacter(exprAst.getTokenValue());
} else if (nodeType.equals(GenericTokenType.IDENTIFIER)) {
String value = preprocessor.valueOf(exprAst.getTokenValue());
return value == null ? BigInteger.ZERO : evalToInt(value, exprAst);

final String id = exprAst.getTokenValue();
if (macroEvaluationStack.contains(id))
{
if (LOG.isDebugEnabled())
{
LOG.debug("ExpressionEvaluator: self-referential macro '{}' detected; assume true; evaluation stack = ['{} <- {}']", id, id, String.join(" <- ", macroEvaluationStack));
}
return BigInteger.ONE;
}
final String value = preprocessor.valueOf(id);
if (value == null)
{
return BigInteger.ZERO;
}

macroEvaluationStack.addFirst(id);
BigInteger expansion = evalToInt(value, exprAst);
macroEvaluationStack.removeFirst();
return expansion;

} else {
throw new EvaluationException("Unknown expression type '" + nodeType + "'");
}
Expand Down Expand Up @@ -194,21 +229,21 @@ private static AstNode getNextOperand(@Nullable AstNode node) {
// ////////////// logical expressions ///////////////////////////
private BigInteger evalLogicalOrExpression(AstNode exprAst) {
AstNode operand = exprAst.getFirstChild();
boolean result = eval(operand);
boolean result = evalToBoolean(operand);

while (!result && ((operand = getNextOperand(operand)) != null)) {
result = eval(operand);
result = evalToBoolean(operand);
}

return result ? BigInteger.ONE : BigInteger.ZERO;
}

private BigInteger evalLogicalAndExpression(AstNode exprAst) {
AstNode operand = exprAst.getFirstChild();
boolean result = eval(operand);
boolean result = evalToBoolean(operand);

while (result && ((operand = getNextOperand(operand)) != null)) {
result = eval(operand);
result = evalToBoolean(operand);
}

return result ? BigInteger.ONE : BigInteger.ZERO;
Expand All @@ -233,9 +268,9 @@ private BigInteger evalEqualityExpression(AstNode exprAst) {
operatorType = operator.getType();
rhs = operator.getNextSibling();
if (operatorType.equals(CppPunctuator.EQ)) {
result = result == eval(rhs);
result = result == evalToBoolean(rhs);
} else if (operatorType.equals(CppPunctuator.NOT_EQ)) {
result = result != eval(rhs);
result = result != evalToBoolean(rhs);
} else {
throw new EvaluationException("Unknown equality operator '" + operatorType + "'");
}
Expand Down Expand Up @@ -332,7 +367,7 @@ private BigInteger evalUnaryExpression(AstNode exprAst) {
} else if (operatorType.equals(CppPunctuator.MINUS)) {
return evalToInt(operand).negate();
} else if (operatorType.equals(CppPunctuator.NOT)) {
boolean result = !eval(operand);
boolean result = !evalToBoolean(operand);
return result ? BigInteger.ONE : BigInteger.ZERO;
} else if (operatorType.equals(CppPunctuator.BW_NOT)) {
//todo: need more information (signed/unsigned, data type length) to invert bits in all cases correct
Expand Down Expand Up @@ -414,7 +449,7 @@ private BigInteger evalConditionalExpression(AstNode exprAst) {
AstNode trueCaseOperand = operator.getNextSibling();
operator = trueCaseOperand.getNextSibling();
AstNode falseCaseOperand = operator.getNextSibling();
return eval(decisionOperand) ? evalToInt(trueCaseOperand) : evalToInt(falseCaseOperand);
return evalToBoolean(decisionOperand) ? evalToInt(trueCaseOperand) : evalToInt(falseCaseOperand);
} else {
AstNode decisionOperand = exprAst.getFirstChild();
AstNode operator = decisionOperand.getNextSibling();
Expand Down
Loading