From a8bb4b9e8daf9dc5d8578346b23152136ff88a50 Mon Sep 17 00:00:00 2001 From: Pandurang Patil Date: Thu, 28 Sep 2023 20:13:40 +0530 Subject: [PATCH] Fix for issue #3698 `:=` operator wasn't creating multiple assignment call nodes if more than one variable is declared along with assignment. Fixed the respective use case as well as added few more respective unit tests to cover the use case. --- .../AstForGenDeclarationCreator.scala | 19 +++--- .../astcreation/AstForPrimitivesCreator.scala | 20 ++++--- .../astcreation/AstForStatementsCreator.scala | 59 ++++++++++--------- .../dataflow/ParameterInDataFlowTests.scala | 32 ++++++++++ .../passes/ast/AssignmentOperatorTests.scala | 49 +++++++++++++++ 5 files changed, 137 insertions(+), 42 deletions(-) create mode 100644 joern-cli/frontends/gosrc2cpg/src/test/scala/io/joern/go2cpg/passes/ast/AssignmentOperatorTests.scala diff --git a/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/astcreation/AstForGenDeclarationCreator.scala b/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/astcreation/AstForGenDeclarationCreator.scala index 00cb8c007455..a57515979470 100644 --- a/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/astcreation/AstForGenDeclarationCreator.scala +++ b/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/astcreation/AstForGenDeclarationCreator.scala @@ -54,7 +54,7 @@ trait AstForGenDeclarationCreator(implicit withSchemaValidation: ValidationMode) (valueSpec.json(ParserKeys.Names).arr.toList zip valueSpec.json(ParserKeys.Values).arr.toList) .map { case (lhs, rhs) => (createParserNodeInfo(lhs), createParserNodeInfo(rhs)) } .map { case (lhsParserNode, rhsParserNode) => - astForAssignmentCallNode(lhsParserNode, rhsParserNode, typeFullName) + astForAssignmentCallNode(lhsParserNode, rhsParserNode, typeFullName, valueSpec.code) } .unzip localAsts ++: assCallAsts @@ -70,10 +70,11 @@ trait AstForGenDeclarationCreator(implicit withSchemaValidation: ValidationMode) } - private def astForAssignmentCallNode( + protected def astForAssignmentCallNode( lhsParserNode: ParserNodeInfo, rhsParserNode: ParserNodeInfo, - typeFullName: Option[String] + typeFullName: Option[String], + code: String ): (Ast, Ast) = { val rhsAst = astForBooleanLiteral(rhsParserNode) val rhsTypeFullName = typeFullName.getOrElse(getTypeFullNameFromAstNode(rhsAst)) @@ -82,7 +83,7 @@ trait AstForGenDeclarationCreator(implicit withSchemaValidation: ValidationMode) val arguments = lhsAst ++: rhsAst val cNode = callNode( rhsParserNode, - lhsParserNode.code + rhsParserNode.code, + code, Operators.assignment, Operators.assignment, DispatchTypes.STATIC_DISPATCH, @@ -94,8 +95,12 @@ trait AstForGenDeclarationCreator(implicit withSchemaValidation: ValidationMode) protected def astForLocalNode(localParserNode: ParserNodeInfo, typeFullName: Option[String]): Ast = { val name = localParserNode.json(ParserKeys.Name).str - val node = localNode(localParserNode, name, localParserNode.code, typeFullName.getOrElse(Defines.anyTypeName)) - scope.addToScope(name, (node, typeFullName.getOrElse(Defines.anyTypeName))) - Ast(node) + if name != "_" then { + val node = localNode(localParserNode, name, localParserNode.code, typeFullName.getOrElse(Defines.anyTypeName)) + scope.addToScope(name, (node, typeFullName.getOrElse(Defines.anyTypeName))) + Ast(node) + } else { + Ast() + } } } diff --git a/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/astcreation/AstForPrimitivesCreator.scala b/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/astcreation/AstForPrimitivesCreator.scala index 20c2e2f10b27..c6fccbd21df5 100644 --- a/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/astcreation/AstForPrimitivesCreator.scala +++ b/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/astcreation/AstForPrimitivesCreator.scala @@ -62,14 +62,18 @@ trait AstForPrimitivesCreator(implicit withSchemaValidation: ValidationMode) { t private def astForIdentifier(ident: ParserNodeInfo): Ast = { val identifierName = ident.json(ParserKeys.Name).str - val variableOption = scope.lookupVariable(identifierName) - variableOption match { - case Some((variable, variableTypeName)) => - val node = identifierNode(ident, identifierName, ident.code, variableTypeName) - Ast(node).withRefEdge(node, variable) - case _ => - // TODO: something is wrong here. Refer to SwitchTests -> "be correct for switch case 4" - Ast(identifierNode(ident, identifierName, ident.json(ParserKeys.Name).str, Defines.anyTypeName)) + if identifierName != "_" then { + val variableOption = scope.lookupVariable(identifierName) + variableOption match { + case Some((variable, variableTypeName)) => + val node = identifierNode(ident, identifierName, ident.code, variableTypeName) + Ast(node).withRefEdge(node, variable) + case _ => + // TODO: something is wrong here. Refer to SwitchTests -> "be correct for switch case 4" + Ast(identifierNode(ident, identifierName, ident.json(ParserKeys.Name).str, Defines.anyTypeName)) + } + } else { + Ast() } } diff --git a/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/astcreation/AstForStatementsCreator.scala b/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/astcreation/AstForStatementsCreator.scala index 8810d2ee7690..f66f112fa501 100644 --- a/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/astcreation/AstForStatementsCreator.scala +++ b/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/astcreation/AstForStatementsCreator.scala @@ -64,45 +64,50 @@ trait AstForStatementsCreator(implicit withSchemaValidation: ValidationMode) { t } private def astForAssignStatement(assignStmt: ParserNodeInfo): Seq[Ast] = { - val op = assignStmt.json(ParserKeys.Tok).value match { - case "=" => Operators.assignment - case ":=" => Operators.assignment - case "*=" => Operators.assignmentMultiplication - case "/=" => Operators.assignmentDivision - case "%=" => Operators.assignmentModulo - case "+=" => Operators.assignmentPlus - case "-=" => Operators.assignmentMinus - case "<<=" => Operators.assignmentShiftLeft - case ">>=" => Operators.assignmentArithmeticShiftRight - case "&=" => Operators.assignmentAnd - case "^=" => Operators.assignmentXor - case "|=" => Operators.assignmentOr - case _ => Operator.unknown + assignStmt.json(ParserKeys.Tok).value match { + case "=" => astForOnlyAssignmentOperator(assignStmt, Operators.assignment) + case ":=" => astForDeclrationAssignment(assignStmt, Operators.assignment) + case "*=" => astForOnlyAssignmentOperator(assignStmt, Operators.assignmentMultiplication) + case "/=" => astForOnlyAssignmentOperator(assignStmt, Operators.assignmentDivision) + case "%=" => astForOnlyAssignmentOperator(assignStmt, Operators.assignmentModulo) + case "+=" => astForOnlyAssignmentOperator(assignStmt, Operators.assignmentPlus) + case "-=" => astForOnlyAssignmentOperator(assignStmt, Operators.assignmentMinus) + case "<<=" => astForOnlyAssignmentOperator(assignStmt, Operators.assignmentShiftLeft) + case ">>=" => astForOnlyAssignmentOperator(assignStmt, Operators.assignmentArithmeticShiftRight) + case "&=" => astForOnlyAssignmentOperator(assignStmt, Operators.assignmentAnd) + case "^=" => astForOnlyAssignmentOperator(assignStmt, Operators.assignmentXor) + case "|=" => astForOnlyAssignmentOperator(assignStmt, Operators.assignmentOr) + case _ => astForOnlyAssignmentOperator(assignStmt, Operator.unknown) } + } + private def astForDeclrationAssignment(assignStmt: ParserNodeInfo, op: String): Seq[Ast] = { + val (assCallAsts, localAsts) = + (assignStmt.json(ParserKeys.Lhs).arr.toList zip assignStmt.json(ParserKeys.Rhs).arr.toList) + .map { case (lhs, rhs) => (createParserNodeInfo(lhs), createParserNodeInfo(rhs)) } + .map { case (lhsParserNode, rhsParserNode) => + astForAssignmentCallNode(lhsParserNode, rhsParserNode, None, assignStmt.code) + } + .unzip + localAsts ++: assCallAsts + } + + private def astForOnlyAssignmentOperator(assignStmt: ParserNodeInfo, op: String): Seq[Ast] = { val rhsAst = assignStmt .json(ParserKeys.Rhs) .arr .map(createParserNodeInfo) .flatMap(astForBooleanLiteral) val typeFullName = Some(getTypeFullNameFromAstNode(rhsAst.toSeq)) - val (lhsAst, localAst) = assignStmt + val lhsAst = assignStmt .json(ParserKeys.Lhs) .arr - .map(lhsnode => { - val lhs = createParserNodeInfo(lhsnode) - // create corresponding local node as this is known as short variable declaration operator - val localNode = - if (assignStmt.json(ParserKeys.Tok).value == ":=") then Seq(astForLocalNode(lhs, typeFullName)) - else Seq.empty - val lhsAst = astForNode(lhs) - (lhsAst, localNode) - }) - .unzip - val arguments = lhsAst.flatten ++: rhsAst + .flatMap(astForNode) + + val arguments = lhsAst ++: rhsAst val cNode = callNode(assignStmt, assignStmt.code, op, op, DispatchTypes.STATIC_DISPATCH, None, typeFullName) val callAst_ = Seq(callAst(cNode, arguments.toSeq)) - localAst.flatten ++: callAst_ + callAst_ } private def astForIncDecStatement(incDecStatement: ParserNodeInfo): Ast = { diff --git a/joern-cli/frontends/gosrc2cpg/src/test/scala/io/joern/go2cpg/dataflow/ParameterInDataFlowTests.scala b/joern-cli/frontends/gosrc2cpg/src/test/scala/io/joern/go2cpg/dataflow/ParameterInDataFlowTests.scala index fac8804b892c..cbaad868881a 100644 --- a/joern-cli/frontends/gosrc2cpg/src/test/scala/io/joern/go2cpg/dataflow/ParameterInDataFlowTests.scala +++ b/joern-cli/frontends/gosrc2cpg/src/test/scala/io/joern/go2cpg/dataflow/ParameterInDataFlowTests.scala @@ -20,4 +20,36 @@ class ParameterInDataFlowTests extends GoCodeToCpgSuite(withOssDataflow = true) sink.reachableByFlows(source).size shouldBe 1 } } + + "Simple parameter in to call argument data flow use case" should { + val cpg = code(""" + |package main + |func foo(argv string) { + | sample("sample", argv) + | var a, _ = sample("sample", argv) + | b, _ := sample("sample", argv) + | c, d := sample("sample", argv) + |} + |""".stripMargin) + + "data flow from parameterIn to identifier" in { + val source = cpg.parameter("argv").l + val sink = cpg.identifier("argv").l + sink.reachableByFlows(source).size shouldBe 4 + } + + "data flow from parameterIn to CALL Node" in { + val source = cpg.parameter("argv").l + val sink = cpg.call("sample").l + sink.reachableByFlows(source).size shouldBe 4 + } + + "data flow from parameterIn to lhs varaible" in { + val source = cpg.parameter("argv").l + source.size shouldBe 1 + val sink = cpg.identifier("[a|b|c]").l + sink.size shouldBe 3 + sink.reachableByFlows(source).size shouldBe 3 + } + } } diff --git a/joern-cli/frontends/gosrc2cpg/src/test/scala/io/joern/go2cpg/passes/ast/AssignmentOperatorTests.scala b/joern-cli/frontends/gosrc2cpg/src/test/scala/io/joern/go2cpg/passes/ast/AssignmentOperatorTests.scala new file mode 100644 index 000000000000..66ee758ce733 --- /dev/null +++ b/joern-cli/frontends/gosrc2cpg/src/test/scala/io/joern/go2cpg/passes/ast/AssignmentOperatorTests.scala @@ -0,0 +1,49 @@ +package io.joern.go2cpg.passes.ast + +import io.joern.dataflowengineoss.language.* +import io.joern.go2cpg.testfixtures.GoCodeToCpgSuite +import io.shiftleft.codepropertygraph.generated.Operators +import io.shiftleft.semanticcpg.language.* + +class AssignmentOperatorTests extends GoCodeToCpgSuite { + "Multiple varables declared and assigned with value in single statement" should { + val cpg = code(""" + |package main + |func main() { + | var a, b = "first", "second" + | c, d := 10, "forth" + |} + | + |""".stripMargin) + "Check CALL Nodes" in { + cpg.call(Operators.assignment).size shouldBe 4 + val List(a, b, c, d) = cpg.call(Operators.assignment).l + a.typeFullName shouldBe "string" + b.typeFullName shouldBe "string" + c.typeFullName shouldBe "int" + d.typeFullName shouldBe "string" + } + } + + "Multiple varables declared and assigned with value in single statement with one function call" should { + val cpg = code(""" + |package main + |func foo() int{ + | return 0 + |} + |func main() { + | var a, b = "first", "second" + | c, d, e := 10, "forth", foo() + |} + |""".stripMargin) + "Check CALL Nodes" in { + cpg.call(Operators.assignment).size shouldBe 5 + val List(a, b, c, d, e) = cpg.call(Operators.assignment).l + a.typeFullName shouldBe "string" + b.typeFullName shouldBe "string" + c.typeFullName shouldBe "int" + d.typeFullName shouldBe "string" + e.typeFullName shouldBe "int" + } + } +}