Repository: avast/yaramod (language: python)

Issue:
fix(Rule): Delete tags from the rule
Currently, after we delete tags from the rule, we would still have a `:` with empty tag list. Which is a incorrect syntax. For example:
```yara
rule foo : bar {
    condition:
        true
}
```
will become (which should not happen)
```yara
rule foo : {
    condition:
        true
}
```

This patch refactors the `Rule::setTags` method to always delete the old tags if present. And then add new tags only if necessary.


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5509bf84..a1d6e8ef 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Changelog
 
+# v3.20.2 (2023-09-18)
+
+* fix: Deletion of tags no longer leaves trailing `:` ([#254](https://github.com/avast/yaramod/pull/254))
+* fix: Fixed missing attributes in `dotnet` module
+
 # v3.20.1 (2023-06-20)
 
 * fix: Correct the type of `dotnet.guids` to array ([#248](https://github.com/avast/yaramod/pull/248))
diff --git a/include/yaramod/yaramod.h b/include/yaramod/yaramod.h
index 666e1a83..3daf3be0 100644
--- a/include/yaramod/yaramod.h
+++ b/include/yaramod/yaramod.h
@@ -11,7 +11,7 @@
 
 #define YARAMOD_VERSION_MAJOR 3
 #define YARAMOD_VERSION_MINOR 20
-#define YARAMOD_VERSION_PATCH 1
+#define YARAMOD_VERSION_PATCH 2
 #define YARAMOD_VERSION_ADDEND ""
 
 #define YARAMOD_VERSION STR(YARAMOD_VERSION_MAJOR) "." STR(YARAMOD_VERSION_MINOR) "." STR(YARAMOD_VERSION_PATCH) YARAMOD_VERSION_ADDEND
diff --git a/modules/module_dotnet.json b/modules/module_dotnet.json
index 8a2d06c6..a6ce9ebd 100644
--- a/modules/module_dotnet.json
+++ b/modules/module_dotnet.json
@@ -63,6 +63,195 @@
             "documentation": "The number of GUIDs in the guids array.",
             "type": "i"
         },
+        {
+            "kind": "value",
+            "name": "number_of_classes",
+            "documentation": "The number of classes in the file.",
+            "type": "i"
+        },
+        {
+            "kind": "array",
+            "name": "classes",
+            "documentation": "A zero-based array of .NET classes stored in the metadata. Individual classes can be accessed using the [] operator.",
+            "structure":
+            {
+                "kind": "struct",
+                "name": "classes",
+                "documentation": "",
+                "attributes": [
+                    {
+                        "kind": "value",
+                        "name": "fullname",
+                        "documentation": "Class full name.",
+                        "type": "s"
+                    },
+                    {
+                        "kind": "value",
+                        "name": "name",
+                        "documentation": "Class name.",
+                        "type": "s"
+                    },
+                    {
+                        "kind": "value",
+                        "name": "namespace",
+                        "documentation": "Class namespace.",
+                        "type": "s"
+                    },
+                    {
+                        "kind": "value",
+                        "name": "visibility",
+                        "documentation": "Class visibility specifier, options are: private, public, protected, internal, private protected and protected internal",
+                        "type" : "s"
+                    },
+                    {
+                        "kind": "value",
+                        "name": "type",
+                        "documentation": "Type of the object, options are: class and interface",
+                        "type" : "s"
+                    },
+                    {
+                        "kind": "value",
+                        "name": "abstract",
+                        "documentation": "Boolean representing if class is abstract.",
+                        "type": "i"
+                    },
+                    {
+                        "kind": "value",
+                        "name": "sealed",
+                        "documentation": "Boolean representing if class is sealed.",
+                        "type": "i"
+                    },
+                    {
+                        "kind": "value",
+                        "name": "number_of_generic_parameters",
+                        "documentation": "Number of generic parameters.",
+                        "type": "i"
+                    },
+                    {
+                        "kind": "array",
+                        "name": "generic_parameters",
+                        "documentation": "A zero-based array of generic parameters name. Individual parameters can be accessed using the [] operator.",
+                        "type": "s"
+                    },
+                    {
+                        "kind": "value",
+                        "name": "number_of_base_types",
+                        "documentation": "Number of the base types.",
+                        "type": "i"
+                    },
+                    {
+                        "kind": "array",
+                        "name": "base_types",
+                        "documentation": "A zero-based array of base types name. Individual base types can be accessed using the [] operator.",
+                        "type": "s"
+                    },
+                    {
+                        "kind": "value",
+                        "name": "number_of_methods",
+                        "documentation": "Number of the methods.",
+                        "type": "i"
+                    },
+                    {
+                        "kind": "array",
+                        "name": "methods",
+                        "documentation": "A zero-based array of method objects. Individual methods can be accessed by using the [] operator.",
+                        "structure":
+                        {
+                            "kind": "struct",
+                            "name": "methods",
+                            "documentation": "",
+                            "attributes": [
+                                {
+                                    "kind": "value",
+                                    "name": "name",
+                                    "documentation": "Method name.",
+                                    "type": "s"
+                                },
+                                {
+                                    "kind": "value",
+                                    "name": "visibility",
+                                    "documentation": "Method visibility specifier, options are: private, public, protected, internal, private protected and protected internal",
+                                    "type" : "s"
+                                },
+                                {
+                                    "kind": "value",
+                                    "name": "static",
+                                    "documentation": "Boolean representing if method is static.",
+                                    "type": "i"
+                                },
+                                {
+                                    "kind": "value",
+                                    "name": "virtual",
+                                    "documentation": "Boolean representing if method is virtual.",
+                                    "type": "i"
+                                },
+                                {
+                                    "kind": "value",
+                                    "name": "final",
+                                    "documentation": "Boolean representing if method is final.",
+                                    "type": "i"
+                                },
+                                {
+                                    "kind": "value",
+                                    "name": "abstract",
+                                    "documentation": "Boolean representing if method is abstract.",
+                                    "type": "i"
+                                },
+                                {
+                                    "kind": "value",
+                                    "name": "return_type",
+                                    "documentation": "Method return type name.",
+                                    "type": "s"
+                                },
+                                {
+                                    "kind": "value",
+                                    "name": "number_of_parameters",
+                                    "documentation": "Number of the method parameters.",
+                                    "type": "i"
+                                },
+                                {
+                                    "kind": "array",
+                                    "name": "parameters",
+                                    "documentation": "A zero-based array of method parameters. Individual parameters can be accessed by using the [] operator.",
+                                    "structure":
+                                    {
+                                        "kind": "struct",
+                                        "name": "parameters",
+                                        "documentation": "",
+                                        "attributes": [
+                                            {
+                                                "kind": "value",
+                                                "name": "name",
+                                                "documentation": "Parameter name.",
+                                                "type": "s"
+                                            },
+                                            {
+                                                "kind": "value",
+                                                "name": "type",
+                                                "documentation": "Parameter type.",
+                                                "type": "s"
+                                            }
+                                        ]
+                                    }
+                                },
+                                {
+                                    "kind": "value",
+                                    "name": "number_of_generic_parameters",
+                                    "documentation": "Number of the method generic parameters.",
+                                    "type": "i"
+                                },
+                                {
+                                    "kind": "array",
+                                    "name": "generic_parameters",
+                                    "documentation": "A zero-based array of method generic parameters. Individual parameters can be accessed by using the [] operator.",
+                                    "type": "s"
+                                }
+                            ]
+                        }
+                    }
+                ]
+            }
+        },
         {
             "kind": "array",
             "name": "resources",
diff --git a/src/types/rule.cpp b/src/types/rule.cpp
index 210cc98c..ea8871d7 100644
--- a/src/types/rule.cpp
+++ b/src/types/rule.cpp
@@ -343,27 +343,42 @@ void Rule::setStringsTrie(const std::shared_ptr<StringsTrie>&& strings)
  */
 void Rule::setTags(const std::vector<std::string>& tags)
 {
-	TokenIt insert_before;
-	if (_tags.empty())
-	{
-		insert_before = std::find_if(_name, _tokenStream->end(),
-			[](const Token& t){ return t.getType() == TokenType::NEW_LINE || t.getType() == TokenType::RULE_BEGIN; }
-			);
-		assert(insert_before != _tokenStream->end() && "Called setTags on rule that does not contain '{'");
-		_tokenStream->emplace(insert_before, TokenType::COLON, ":");
-	}
-	else
+	// If the rule contains tags, we remove them, so that we are in a clean
+	// state before setting new tags
+	if (!_tags.empty())
 	{
-		//delete all tags from tokenStream
+		TokenIt colon = std::find_if(_name, _tokenStream->end(),
+			[](const Token& t){ return t.getType() == TokenType::COLON; }
+		);
+		assert(colon != _tokenStream->end() && "Called setTags on rule that does not contain ':' but has tags");
+		_tokenStream->erase(colon);
+
+		// Delete all current tags from TokenStream
 		for (const TokenIt& it : _tags)
-			insert_before = _tokenStream->erase(it);
+		_tokenStream->erase(it);
+
+		// Discard the current tags
+		_tags = std::vector<TokenIt>();
 	}
-	_tags = std::vector<TokenIt>();
-	// Insert new tags into TokenStream
-	for (const auto& tag : tags)
+
+	// If we want to set new tags, generate tokens for them
+	if (!tags.empty())
 	{
-		TokenIt tagIt = _tokenStream->insert(insert_before, TokenType::TAG, Literal(tag));
-		_tags.push_back(tagIt);
+		// Try to find where to place the tags
+		TokenIt insert_before = std::find_if(_name, _tokenStream->end(),
+			[](const Token& t){ return t.getType() == TokenType::NEW_LINE || t.getType() == TokenType::RULE_BEGIN; }
+		);
+		assert(insert_before != _tokenStream->end() && "Called setTags on rule that does not contain '{'");
+
+		// Generate `:` to separate rule name and tags
+		_tokenStream->emplace(insert_before, TokenType::COLON, ":");
+
+		// Insert the new tags into TokenStream
+		for (const auto& tag : tags)
+		{
+			TokenIt tagIt = _tokenStream->insert(insert_before, TokenType::TAG, Literal(tag));
+			_tags.push_back(tagIt);
+		}
 	}
 }
 


Test patch:
diff --git a/tests/cpp/parser_tests.cpp b/tests/cpp/parser_tests.cpp
index 51d16003..83e1cef1 100644
--- a/tests/cpp/parser_tests.cpp
+++ b/tests/cpp/parser_tests.cpp
@@ -3533,8 +3533,63 @@ import "dotnet"
 rule dotnet_module
 {
 	condition:
+		dotnet.is_dotnet and
+		dotnet.version == "v2.0.50727" and
+		dotnet.module_name == "axs" and
+		dotnet.number_of_streams == 123 and
+		dotnet.streams[0].name == "#~" and
+		dotnet.streams[0].offset == 123 and
+		dotnet.streams[0].size == 123 and
+		dotnet.number_of_guids == 123 and
+		dotnet.guids[0] == "99c08ffd-f378-a891-10ab-c02fe11be6ef" and
+		dotnet.number_of_classes == 123 and
+		dotnet.classes[0].fullname == "Launcher.Program" and
+		dotnet.classes[0].name == "Launcher.Program" and
+		dotnet.classes[0].namespace == "test" and
+		dotnet.classes[0].visibility == "private" and
+		dotnet.classes[0].type == "class" and
+		dotnet.classes[0].abstract and
+		dotnet.classes[0].sealed and
+		dotnet.classes[0].number_of_generic_parameters == 123 and
+		dotnet.classes[0].generic_parameters[0] == "test" and
+		dotnet.classes[0].number_of_base_types == 123 and
+		dotnet.classes[0].base_types[0] == "test" and
+		dotnet.classes[0].number_of_methods == 123 and
+		dotnet.classes[0].methods[0].name == "test" and
+		dotnet.classes[0].methods[0].visibility == "public" and
+		dotnet.classes[0].methods[0].static and
+		dotnet.classes[0].methods[0].virtual and
+		dotnet.classes[0].methods[0].final and
+		dotnet.classes[0].methods[0].abstract and
+		dotnet.classes[0].methods[0].return_type == "test" and
+		dotnet.classes[0].methods[0].number_of_parameters == 123 and
+		dotnet.classes[0].methods[0].parameters[0].name == "test" and
+		dotnet.classes[0].methods[0].parameters[0].type == "test" and
+		dotnet.classes[0].methods[0].number_of_generic_parameters == 123 and
+		dotnet.classes[0].methods[0].generic_parameters[0] == "test" and
+		dotnet.number_of_resources == 123 and
+		dotnet.resources[0].offset == 0x4d5a and
+		dotnet.resources[0].length == 0x4d5a and
+		dotnet.resources[0].name == "test" and
+		dotnet.assembly.version.major == 7 and
+		dotnet.assembly.version.minor == 0 and
+		dotnet.assembly.version.build_number == 0 and
+		dotnet.assembly.version.revision_number == 0 and
 		dotnet.assembly.name == "Keylogger" and
-		dotnet.guids[0] == "99c08ffd-f378-a891-10ab-c02fe11be6ef"
+		dotnet.assembly.culture == "test" and
+		dotnet.number_of_modulerefs == 123 and
+		dotnet.modulerefs[0] == "kernel32" and
+		dotnet.typelib == "kernel32" and
+		dotnet.assembly_refs[0].version.major == 7 and
+		dotnet.assembly_refs[0].version.minor == 0 and
+		dotnet.assembly_refs[0].version.build_number == 0 and
+		dotnet.assembly_refs[0].version.revision_number == 0 and
+		dotnet.assembly_refs[0].name == "test" and
+		dotnet.assembly_refs[0].public_key_or_token == "test" and
+		dotnet.number_of_user_strings == 123 and
+		dotnet.user_strings[0] == "test" and
+		dotnet.number_of_field_offsets == 123 and
+		dotnet.field_offsets[0] == 8675309
 }
 )");
 
@@ -3542,9 +3597,9 @@ rule dotnet_module
 	ASSERT_EQ(1u, driver.getParsedFile().getRules().size());
 
 	const auto& rule = driver.getParsedFile().getRules()[0];
-	EXPECT_EQ("dotnet.assembly.name == \"Keylogger\" and dotnet.guids[0] == \"99c08ffd-f378-a891-10ab-c02fe11be6ef\"", rule->getCondition()->getText());
+    EXPECT_EQ("dotnet.is_dotnet and dotnet.version == \"v2.0.50727\" and dotnet.module_name == \"axs\" and dotnet.number_of_streams == 123 and dotnet.streams[0].name == \"#~\" and dotnet.streams[0].offset == 123 and dotnet.streams[0].size == 123 and dotnet.number_of_guids == 123 and dotnet.guids[0] == \"99c08ffd-f378-a891-10ab-c02fe11be6ef\" and dotnet.number_of_classes == 123 and dotnet.classes[0].fullname == \"Launcher.Program\" and dotnet.classes[0].name == \"Launcher.Program\" and dotnet.classes[0].namespace == \"test\" and dotnet.classes[0].visibility == \"private\" and dotnet.classes[0].type == \"class\" and dotnet.classes[0].abstract and dotnet.classes[0].sealed and dotnet.classes[0].number_of_generic_parameters == 123 and dotnet.classes[0].generic_parameters[0] == \"test\" and dotnet.classes[0].number_of_base_types == 123 and dotnet.classes[0].base_types[0] == \"test\" and dotnet.classes[0].number_of_methods == 123 and dotnet.classes[0].methods[0].name == \"test\" and dotnet.classes[0].methods[0].visibility == \"public\" and dotnet.classes[0].methods[0].static and dotnet.classes[0].methods[0].virtual and dotnet.classes[0].methods[0].final and dotnet.classes[0].methods[0].abstract and dotnet.classes[0].methods[0].return_type == \"test\" and dotnet.classes[0].methods[0].number_of_parameters == 123 and dotnet.classes[0].methods[0].parameters[0].name == \"test\" and dotnet.classes[0].methods[0].parameters[0].type == \"test\" and dotnet.classes[0].methods[0].number_of_generic_parameters == 123 and dotnet.classes[0].methods[0].generic_parameters[0] == \"test\" and dotnet.number_of_resources == 123 and dotnet.resources[0].offset == 0x4d5a and dotnet.resources[0].length == 0x4d5a and dotnet.resources[0].name == \"test\" and dotnet.assembly.version.major == 7 and dotnet.assembly.version.minor == 0 and dotnet.assembly.version.build_number == 0 and dotnet.assembly.version.revision_number == 0 and dotnet.assembly.name == \"Keylogger\" and dotnet.assembly.culture == \"test\" and dotnet.number_of_modulerefs == 123 and dotnet.modulerefs[0] == \"kernel32\" and dotnet.typelib == \"kernel32\" and dotnet.assembly_refs[0].version.major == 7 and dotnet.assembly_refs[0].version.minor == 0 and dotnet.assembly_refs[0].version.build_number == 0 and dotnet.assembly_refs[0].version.revision_number == 0 and dotnet.assembly_refs[0].name == \"test\" and dotnet.assembly_refs[0].public_key_or_token == \"test\" and dotnet.number_of_user_strings == 123 and dotnet.user_strings[0] == \"test\" and dotnet.number_of_field_offsets == 123 and dotnet.field_offsets[0] == 8675309", rule->getCondition()->getText());
 	EXPECT_EQ("dotnet", rule->getCondition()->getFirstTokenIt()->getPureText());
-	EXPECT_EQ("99c08ffd-f378-a891-10ab-c02fe11be6ef", rule->getCondition()->getLastTokenIt()->getPureText());
+	EXPECT_EQ("8675309", rule->getCondition()->getLastTokenIt()->getPureText());
 
 	EXPECT_EQ(input_text, driver.getParsedFile().getTextFormatted());
 }
diff --git a/tests/python/test_representation.py b/tests/python/test_representation.py
index 13876cbb..d3d9d44a 100644
--- a/tests/python/test_representation.py
+++ b/tests/python/test_representation.py
@@ -426,3 +426,67 @@ def test_custom_module_enhancing_known_module(self):
         self.assertEqual(version_overloads_names[0], ["version"])
         self.assertEqual(version_overloads_names[1], ["version", "toolid"])
         self.assertEqual(version_overloads_names[2], ["version", "test string argument"])
+
+    def test_set_tags_to_empty_list_on_a_rule_without_tags(self):
+        yara_file = yaramod.Yaramod().parse_string('''rule empty_rule
+{
+	condition:
+		true
+}''')
+        yara_file.rules[0].tags = []
+
+        expected = '''rule empty_rule
+{
+	condition:
+		true
+}
+'''
+        self.assertEqual(expected, yara_file.text_formatted)
+
+    def test_set_tags_to_empty_list(self):
+        yara_file = yaramod.Yaramod().parse_string('''rule empty_rule : test 
+{
+	condition:
+		true
+}''')
+        yara_file.rules[0].tags = []
+
+        expected = '''rule empty_rule
+{
+	condition:
+		true
+}
+'''
+        self.assertEqual(expected, yara_file.text_formatted)
+
+    def test_set_tags_on_a_rule_without_tags(self):
+        yara_file = yaramod.Yaramod().parse_string('''rule empty_rule
+{
+	condition:
+		true
+}''')
+        yara_file.rules[0].tags = ['foo', 'bar']
+
+        expected = '''rule empty_rule : foo bar
+{
+	condition:
+		true
+}
+'''
+        self.assertEqual(expected, yara_file.text_formatted)
+
+    def test_set_tags_on_a_rule_with_tags(self):
+        yara_file = yaramod.Yaramod().parse_string('''rule empty_rule : baz
+{
+	condition:
+		true
+}''')
+        yara_file.rules[0].tags = ['foo', 'bar']
+
+        expected = '''rule empty_rule : foo bar
+{
+	condition:
+		true
+}
+'''
+        self.assertEqual(expected, yara_file.text_formatted)