Add severity and pass message for findings
MODIFY
src/main/java/de/fraunhofer/aisec/codyze/analysis/Finding.java
src/main/java/de/fraunhofer/aisec/codyze/analysis/Finding.java
@@ -4,6 +4,7 @@ package de.fraunhofer.aisec.codyze.analysis;
import com.fasterxml.jackson.annotation.JsonIgnore;
import de.fraunhofer.aisec.cpg.sarif.PhysicalLocation;
import de.fraunhofer.aisec.cpg.sarif.Region;
+import de.fraunhofer.aisec.mark.markDsl.Action;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
@@ -16,17 +17,35 @@ import java.util.stream.Collectors;
/**
* Representation of a vulnerability/non-vulnerability found in the source code.
- *
*/
public class Finding {
- private final String onFailIdentifier;
/**
- * True, if this Finding indicates a problem/vulnerability. False, if this Finding indicates a code snippet that has been checked and verified.
+ * Identifier for this finding.
+ */
+ private String id;
+
+ /**
+ *
+ */
+ private final Action action;
+
+ /**
+ * True, if this finding indicates a problem/vulnerability. False, if this
+ * finding indicates a code snippet that has been checked and verified.
*/
private boolean isProblem = true;
+ /**
+ * Simplified description of this finding. Usually state that a rule was
+ * validated or violated.
+ */
private final String logMsg;
+
+ /**
+ * Location (lines and columns) in source code files, where this finding was
+ * produced.
+ */
@NonNull
private final List<PhysicalLocation> locations = new ArrayList<>();
@@ -35,15 +54,16 @@ public class Finding {
*
* @param logMsg Log message for that specific finding. This message is created by the analysis module and may contain further descriptions and details of the
* finding.
- * @param onfailIdentifier Identifier of the generic finding, as given by the "onfail" construct of the MARK rule.
* @param startLine Line in code where the finding begins. Note that LSP starts counting at 1.
* @param endLine Line in code where the finding ends.
* @param startColumn Column in code where the finding begins. Note that LPS start counting at 1.
* @param endColumn Column in code where the finding ends.
*/
- public Finding(String logMsg, String onfailIdentifier, @Nullable URI artifactUri, int startLine, int endLine, int startColumn, int endColumn) {
+ public Finding(String id, Action action, String logMsg, @Nullable URI artifactUri, int startLine, int endLine, int startColumn,
+ int endColumn) {
+ this.id = id;
+ this.action = action;
this.logMsg = logMsg;
- this.onFailIdentifier = onfailIdentifier;
if (artifactUri != null) {
this.locations
.add(new PhysicalLocation(artifactUri, new Region(startLine, startColumn, endLine, endColumn)));
@@ -53,16 +73,18 @@ public class Finding {
/**
* Constructor.
*
+ * @param id
* @param logMsg Log message for that specific finding. This message is created by the analysis module and may contain further descriptions and details of the
* finding.
* @param artifactUri Absolute URI of the source file.
- * @param onfailIdentifier Identifier of the generic finding, as given by the "onfail" construct of the MARK rule.
* @param ranges List of LSP "ranges" determining the position(s) in code of this finding. Note that a LSP range starts counting at 1, while a CPG "region" starts
* @param isProblem true, if this Finding represents a vulnerability/weakness. False, if the Finding confirms that the code is actually correct.
*/
- public Finding(String logMsg, @Nullable URI artifactUri, String onfailIdentifier, List<Region> ranges, boolean isProblem) {
+ public Finding(String id, Action action, String logMsg, @Nullable URI artifactUri, List<Region> ranges, boolean isProblem) {
+ this.id = id;
+ this.action = action;
this.logMsg = logMsg;
- this.onFailIdentifier = onfailIdentifier;
+
for (Region r : ranges) {
if (artifactUri != null) {
this.locations.add(new PhysicalLocation(artifactUri, r));
@@ -71,6 +93,14 @@ public class Finding {
this.isProblem = isProblem;
}
+ public String getIdentifier() {
+ return id;
+ }
+
+ public Action getAction() {
+ return action;
+ }
+
public String getLogMsg() {
return logMsg;
}
@@ -92,7 +122,7 @@ public class Finding {
/**
* Returns a non-null, possibly empty list of physical locations of this finding.
- *
+ * <p>
* This method is required for a proper result JSON.
*
* @return
@@ -102,20 +132,14 @@ public class Finding {
return this.locations;
}
- public String getOnfailIdentifier() {
- return onFailIdentifier;
- }
-
public String toString() {
String addIfExists = "";
- if (onFailIdentifier != null) {
// simple for now
- String descriptionShort = FindingDescription.getInstance().getDescriptionShort(onFailIdentifier);
- if (descriptionShort != null && !descriptionShort.equals(onFailIdentifier)) {
+ String descriptionShort = FindingDescription.getInstance().getDescriptionShort(id);
+ if (descriptionShort != null && !descriptionShort.equals(id)) {
addIfExists = ": " + descriptionShort;
}
- }
String lines;
if (locations.size() == 1) {
@@ -130,7 +154,8 @@ public class Finding {
if (!(obj instanceof Finding)) {
return false;
}
- return Objects.equals(this.logMsg, ((Finding) obj).logMsg) && Objects.equals(this.onFailIdentifier, ((Finding) obj).onFailIdentifier);
+ return Objects.equals(this.logMsg, ((Finding) obj).logMsg)
+ && Objects.equals(this.id, ((Finding) obj).id);
}
public int hashCode() {
@@ -143,9 +168,9 @@ public class Finding {
* @param out
*/
public void prettyPrintShort(@NonNull PrintStream out) {
- String shortMsg = FindingDescription.getInstance().getDescriptionShort(onFailIdentifier);
+ String shortMsg = FindingDescription.getInstance().getDescriptionShort(id);
if (shortMsg == null) {
- shortMsg = onFailIdentifier;
+ shortMsg = id;
}
String lines;
@@ -157,4 +182,5 @@ public class Finding {
out.println(lines + ": " + (isProblem ? "(BAD) " : "(GOOD) ") + shortMsg + ": " + logMsg);
}
+
}

MODIFY
src/main/java/de/fraunhofer/aisec/codyze/analysis/FindingDescription.kt
src/main/java/de/fraunhofer/aisec/codyze/analysis/FindingDescription.kt
@@ -14,30 +14,35 @@ class FindingDescription private constructor() {
val helpUri: String?,
val fullDescription: Description?,
val shortDescription: Description?,
+ val passMessage: Description?,
val fixes: List<Fix>?
)
class Fix(val description: Description)
class Description(val text: String)
- fun get(onFailId: String): FindingDescriptionItem? {
- return items?.get(onFailId)
+ fun get(id: String): FindingDescriptionItem? {
+ return items?.get(id)
}
- fun getDescriptionFull(onFailId: String): String? {
- return items?.get(onFailId)?.fullDescription?.text
+ fun getDescriptionFull(id: String): String? {
+ return items?.get(id)?.fullDescription?.text
}
- fun getDescriptionShort(onFailId: String): String? {
- return items?.get(onFailId)?.shortDescription?.text
+ fun getDescriptionShort(id: String): String? {
+ return items?.get(id)?.shortDescription?.text
}
- fun getHelpUri(onFailId: String): String? {
- return items?.get(onFailId)?.helpUri
+ fun getDescriptionPass(id: String): String? {
+ return items?.get(id)?.passMessage?.text
}
- fun getFixes(onFailId: String): List<String>? {
- return items?.get(onFailId)?.fixes?.map { it.description.text }
+ fun getHelpUri(id: String): String? {
+ return items?.get(id)?.helpUri
+ }
+
+ fun getFixes(id: String): List<String>? {
+ return items?.get(id)?.fixes?.map { it.description.text }
}
fun init(descriptionFile: File) {

MODIFY
src/main/java/de/fraunhofer/aisec/codyze/analysis/markevaluation/Evaluator.java
src/main/java/de/fraunhofer/aisec/codyze/analysis/markevaluation/Evaluator.java
@@ -252,14 +252,17 @@ public class Evaluator {
}
boolean isRuleViolated = !(Boolean) evaluationResultUb;
+
findings.add(new Finding(
+ rule.getErrorMessage() != null ? rule.getErrorMessage() : rule.getName(),
+ rule.getStatement().getAction(),
"Rule "
+ rule.getName()
+ (isRuleViolated ? " violated" : " verified"),
currentFile,
- rule.getErrorMessage(),
ranges,
isRuleViolated));
+
}
} else if (evaluationResultUb == null) {
log.warn("Unable to evaluate rule {} in MARK context " + markCtx + "/" + markCtxHolder.getAllContexts().size()
@@ -353,7 +356,6 @@ public class Evaluator {
* In that case, the function will return [ [ (c1, v1) , (c1, v2), (c1, v3) ] [ (c2, c1), (c2, v2), (c2, v3) ] ]
*
* @param rule the MARK rule
- *
* @return
*/
protected List<List<Pair<String, Node>>> findInstancesForEntities(MRule rule) {

MODIFY
src/main/java/de/fraunhofer/aisec/codyze/analysis/markevaluation/ExpressionEvaluator.java
src/main/java/de/fraunhofer/aisec/codyze/analysis/markevaluation/ExpressionEvaluator.java
@@ -128,9 +128,10 @@ public class ExpressionEvaluator {
if (node != null) {
var ranges = List.of(Utils.getRegionByNode(node));
Finding f = new Finding(
+ (this.markRule.getErrorMessage() != null ? this.markRule.getErrorMessage() : this.markRule.getName()),
+ this.markRule.getStatement().getAction(),
"Verified Order: " + this.markRule.getName(),
new File(node.getFile()).toURI(),
- "",
ranges,
false);
this.resultCtx.getFindings().add(f);

MODIFY
src/main/java/de/fraunhofer/aisec/codyze/analysis/markevaluation/ForbiddenEvaluator.kt
src/main/java/de/fraunhofer/aisec/codyze/analysis/markevaluation/ForbiddenEvaluator.kt
@@ -5,6 +5,7 @@ import de.fraunhofer.aisec.codyze.analysis.Finding
import de.fraunhofer.aisec.codyze.analysis.utils.Utils
import de.fraunhofer.aisec.codyze.markmodel.MOp
import de.fraunhofer.aisec.codyze.markmodel.Mark
+import de.fraunhofer.aisec.mark.markDsl.Action
import de.fraunhofer.aisec.mark.markDsl.OpStatement
import java.io.File
import org.slf4j.LoggerFactory
@@ -65,8 +66,9 @@ class ForbiddenEvaluator(private val markModel: Mark) {
node.code)
val f =
Finding(
+ "FORBIDDEN_" + entity.name,
+ Action.FAIL,
message,
- "FORBIDDEN",
File(node.file).toURI(),
region.startLine,
region.endLine,

MODIFY
src/main/java/de/fraunhofer/aisec/codyze/analysis/markevaluation/OrderNFAEvaluator.java
src/main/java/de/fraunhofer/aisec/codyze/analysis/markevaluation/OrderNFAEvaluator.java
@@ -285,6 +285,8 @@ public class OrderNFAEvaluator {
var region = Utils.getRegionByNode(vertex);
Finding f = new Finding(
+ rule.getErrorMessage() != null ? rule.getErrorMessage() : rule.getName(),
+ rule.getStatement().getAction(),
"Violation against Order: "
+ vertex.getCode()
+ " ("
@@ -297,7 +299,6 @@ public class OrderNFAEvaluator {
+ " ("
+ rule.getErrorMessage()
+ ")",
- rule.getErrorMessage(),
new File(vertex.getFile()).toURI(),
region.getStartLine(),
region.getEndLine(),
@@ -442,6 +443,8 @@ public class OrderNFAEvaluator {
endCol = region.getEndColumn();
}
Finding f = new Finding(
+ rule.getErrorMessage() != null ? rule.getErrorMessage() : rule.getName(),
+ rule.getStatement().getAction(),
"Violation against Order: Base "
+ base
+ " is not correctly terminated. Expected one of ["
@@ -453,7 +456,6 @@ public class OrderNFAEvaluator {
+ " ("
+ rule.getErrorMessage()
+ ")",
- rule.getErrorMessage(),
file,
startLine,
endLine,

MODIFY
src/main/java/de/fraunhofer/aisec/codyze/analysis/wpds/TypestateAnalysis.kt
src/main/java/de/fraunhofer/aisec/codyze/analysis/wpds/TypestateAnalysis.kt
@@ -316,9 +316,10 @@ class TypestateAnalysis(private val markContextHolder: MarkContextHolder) {
}
return Finding(
+ rule.errorMessage ?: rule.name,
+ rule.statement.action,
name,
currentFile,
- rule.errorMessage,
listOf(Utils.getRegionByNode(stmt)),
true
)
@@ -336,9 +337,10 @@ class TypestateAnalysis(private val markContextHolder: MarkContextHolder) {
*/
private fun createGoodFinding(node: Node, `val`: Val?, currentFile: URI): Finding {
return Finding(
+ rule.errorMessage ?: rule.name,
+ rule.statement.action,
"Good: $`val` at $node",
currentFile,
- rule.errorMessage,
listOf(Utils.getRegionByNode(node)),
false
)

MODIFY
src/main/java/de/fraunhofer/aisec/codyze/crymlin/connectors/lsp/CpgDocumentService.java
src/main/java/de/fraunhofer/aisec/codyze/crymlin/connectors/lsp/CpgDocumentService.java
@@ -157,20 +157,41 @@ public class CpgDocumentService implements TextDocumentService {
for (Region reg : f.getRegions()) {
Diagnostic diagnostic = new Diagnostic();
// TODO Replace HINT for verified findings with Code Lens
- diagnostic.setSeverity(f.isProblem() ? DiagnosticSeverity.Error : DiagnosticSeverity.Information);
+
+ // Everything is informational until it becomes a problem
+ DiagnosticSeverity severity = DiagnosticSeverity.Information;
+ if (f.isProblem()) {
+ switch (f.getAction()) {
+ case INFO:
+ severity = DiagnosticSeverity.Information;
+ break;
+ case WARN:
+ severity = DiagnosticSeverity.Warning;
+ break;
+ case FAIL:
+ // Error
+ default:
+ severity = DiagnosticSeverity.Error;
+ break;
+ }
+ }
+ diagnostic.setSeverity(severity);
// Get human readable description, if available
- String msg = FindingDescription.getInstance().getDescriptionShort(f.getOnfailIdentifier());
+ String msg = (f.isProblem() ? FindingDescription.getInstance().getDescriptionShort(f.getIdentifier())
+ : FindingDescription.getInstance().getDescriptionPass(f.getIdentifier()));
if (msg == null) {
msg = f.getLogMsg();
}
- String longDesc = FindingDescription.getInstance().getDescriptionFull(f.getOnfailIdentifier());
+ if (f.isProblem()) {
+ String longDesc = FindingDescription.getInstance().getDescriptionFull(f.getIdentifier());
if (longDesc != null) {
msg += ": " + longDesc;
}
+ }
- diagnostic.setCode(Either.forLeft(f.getOnfailIdentifier()));
+ diagnostic.setCode(Either.forLeft(f.getIdentifier()));
diagnostic.setSource("Codyze");
diagnostic.setMessage(msg);
Range r = new Range(
@@ -181,7 +202,7 @@ public class CpgDocumentService implements TextDocumentService {
String line = ignoredLines.get(r.getStart().getLine());
if (line == null
|| (!line.contains(DISABLE_FINDING_ALL)
- && !line.contains(DISABLE_FINDING_PREFIX + f.getOnfailIdentifier()))) {
+ && !line.contains(DISABLE_FINDING_PREFIX + f.getIdentifier()))) {
allDiags.add(diagnostic);
} else {
log.warn("Skipping finding {}, disabled via comment", f);

MODIFY
src/main/java/de/fraunhofer/aisec/codyze/markmodel/MRule.java
src/main/java/de/fraunhofer/aisec/codyze/markmodel/MRule.java
@@ -15,6 +15,7 @@ public class MRule {
private String name;
private RuleStatement statement;
private String errorMessage;
+
/**
* stores Entity-alias to Pair(Name of Entity, EntityReference). The EntityReference can be NULL if the entity is not available/parsed. E.g. for the rule rule
* UseOfBotan_CipherMode { using Order as cm ensure order cm.start(), cm.finish() onfail WrongUseOfBotan_CipherMode } this would store: cm -> Pair(Order, Reference to

MODIFY
src/test/java/de/fraunhofer/aisec/codyze/crymlin/FindingDescriptionTest.kt
src/test/java/de/fraunhofer/aisec/codyze/crymlin/FindingDescriptionTest.kt
@@ -3,6 +3,7 @@ package de.fraunhofer.aisec.codyze.crymlin
import de.fraunhofer.aisec.codyze.analysis.Finding
import de.fraunhofer.aisec.codyze.analysis.FindingDescription.Companion.instance
import de.fraunhofer.aisec.cpg.sarif.Region
+import de.fraunhofer.aisec.mark.markDsl.Action
import java.io.File
import java.net.URI
import java.net.URISyntaxException
@@ -18,10 +19,10 @@ internal class FindingDescriptionTest {
// Create some Finding object
val logMsg = "Variable cm not initialized"
val artifactUri = URI.create("file:///tmp/test.cpp")
- val onFailId = "WrongUseOfBotan_CipherMode"
+ val id = "WrongUseOfBotan_CipherMode"
val regions = listOf(Region(1, 2, 10, 12))
val isProblem = true
- Finding(logMsg, artifactUri, onFailId, regions, isProblem)
+ Finding(id, Action.FAIL, logMsg, artifactUri, regions, isProblem)
// Initialize database with explanations
val fd = instance
@@ -31,15 +32,17 @@ internal class FindingDescriptionTest {
fd.init(File(url.toURI()))
- val item = fd.get(onFailId)
- val fullDescription = fd.getDescriptionFull(onFailId)
- val shortDescription = fd.getDescriptionShort(onFailId)
- val helpUri = fd.getHelpUri(onFailId)
- val fixes = fd.getFixes(onFailId)
+ val item = fd.get(id)
+ val fullDescription = fd.getDescriptionFull(id)
+ val shortDescription = fd.getDescriptionShort(id)
+ val passMessage = fd.getDescriptionPass(id)
+ val helpUri = fd.getHelpUri(id)
+ val fixes = fd.getFixes(id)
assertNotNull(item)
assertEquals("Full description", fullDescription)
assertEquals("The order of called Botan methods is wrong.", shortDescription)
+ assertEquals("Order of Botan methods validated.", passMessage)
assertEquals("https://www.codyze.io/explanations/10", helpUri)
assertEquals(1, fixes?.size)
assertEquals("Just fix it!", fixes?.get(0) ?: "")

MODIFY
src/test/java/de/fraunhofer/aisec/codyze/crymlin/JacksonTest.kt
src/test/java/de/fraunhofer/aisec/codyze/crymlin/JacksonTest.kt
@@ -15,6 +15,6 @@ internal class JacksonTest : AbstractMarkTest() {
val finding = findings.iterator().next()
assertEquals(9, finding.locations[0].region.startLine)
- assertEquals("FORBIDDEN", finding.onfailIdentifier)
+ assertEquals("FORBIDDEN_ObjectMapper", finding.identifier)
}
}

MODIFY
src/test/java/de/fraunhofer/aisec/codyze/crymlin/RealBCTest.kt
src/test/java/de/fraunhofer/aisec/codyze/crymlin/RealBCTest.kt
@@ -19,14 +19,14 @@ internal class RealBCTest : AbstractMarkTest() {
"real-examples/bc/rwedoff.Password-Manager/"
)
for (f in findings) {
- println(" ->" + f.onfailIdentifier + " " + f.regions[0] + " " + f.logMsg)
+ println(" ->" + f.identifier + " " + f.regions[0] + " " + f.logMsg)
}
// We expect three (positive) verification of the algorithm used
val wrongAlgorithmsFindings =
findings
.stream()
- .filter { f: Finding? -> f!!.onfailIdentifier == "Wrong_Algorithm" }
+ .filter { f: Finding? -> f!!.identifier == "Wrong_Algorithm" }
.collect(Collectors.toSet())
assertEquals(3, wrongAlgorithmsFindings.size) // 3 in total
assertEquals(

MODIFY
src/test/java/de/fraunhofer/aisec/codyze/crymlin/RealBotanTest.kt
src/test/java/de/fraunhofer/aisec/codyze/crymlin/RealBotanTest.kt
@@ -46,7 +46,7 @@ internal class RealBotanTest : AbstractMarkTest() {
val correctKeyLength =
findings
.stream()
- .filter { f: Finding? -> f!!.onfailIdentifier == "CorrectPrivateKeyLength" }
+ .filter { f: Finding? -> f!!.identifier == "CorrectPrivateKeyLength" }
.findFirst()
assertTrue(correctKeyLength.isPresent)
assertFalse(correctKeyLength.get().isProblem)
@@ -66,7 +66,7 @@ internal class RealBotanTest : AbstractMarkTest() {
val wrongKeyLength =
findings
.stream()
- .filter { f: Finding? -> f!!.onfailIdentifier == "WrongPrivateKeyLength" }
+ .filter { f: Finding? -> f!!.identifier == "WrongPrivateKeyLength" }
.findFirst()
assertTrue(wrongKeyLength.isPresent)
assertTrue(wrongKeyLength.get().isProblem)
@@ -118,7 +118,7 @@ internal class RealBotanTest : AbstractMarkTest() {
val blockCiphers =
findings
.stream()
- .filter { f: Finding? -> f!!.onfailIdentifier == "WrongBlockCipher" }
+ .filter { f: Finding? -> f!!.identifier == "WrongBlockCipher" }
.filter { f: Finding? -> !f!!.isProblem }
.collect(Collectors.toList())
assertTrue(blockCiphers.stream().anyMatch { f: Finding? -> f!!.regions[0].startLine == 15 })
@@ -129,7 +129,7 @@ internal class RealBotanTest : AbstractMarkTest() {
val keyLengths =
findings
.stream()
- .filter { f: Finding? -> f!!.onfailIdentifier == "BadKeyLength" }
+ .filter { f: Finding? -> f!!.identifier == "BadKeyLength" }
.filter { obj: Finding? -> obj!!.isProblem }
.collect(Collectors.toList())
assertEquals(2, keyLengths.size)

MODIFY
src/test/resources/unittests/findingDescription.json
src/test/resources/unittests/findingDescription.json
"text": "Just fix it!"
}
}
- ]
+ ],
+ "passMessage": {
+ "text": "Order of Botan methods validated."
+ }
}
}

Add severity and pass message for findings (#270)

Co-authored-by: Christian Banse <christian.banse@aisec.fraunhofer.de>