Repository: mitre/caldera
Issue: adversary does not show the proper ability if this has a duplicate name (v3.1)

Hi

We have found the following situation:

1.- We have two abilities with the same name, one imported through the atomic plugin, and other in the data/abilities folder. In particular, is the ability "Use PsExec to execute a command on a remote host"

2.- We have an advesary linked to one of those abilities. But when we click over that ability, what shows is the other ability with the same name. It seems that it is showing the first ability that appears in the abilities select box, so I guess the problem is that is seeking the ability by name in that select box, instead of matching by id

Best regards




Changed files:
tests/services/test_rest_svc.py, app/objects/c_ability.py, tests/objects/test_ability.py, tests/services/test_planning_svc.py

diff --git a/app/objects/c_ability.py b/app/objects/c_ability.py
index 51283cda..04814c63 100644
--- a/app/objects/c_ability.py
+++ b/app/objects/c_ability.py
@@ -1,4 +1,5 @@
 import collections
+import logging
 import uuid
 
 import marshmallow as ma
@@ -90,6 +91,11 @@ class Ability(FirstClassObjectInterface, BaseObject):
     def store(self, ram):
         existing = self.retrieve(ram['abilities'], self.unique)
         if not existing:
+            name_match = [x for x in ram['abilities'] if x.name == self.name]
+            if name_match:
+                self.name = self.name + " (2)"
+                logging.info(f"Collision in ability name detected for {self.ability_id} and {name_match[0].ability_id} "
+                             f"({name_match[0].name}). Modifying name of the second ability to {self.name}...")
             ram['abilities'].append(self)
             return self.retrieve(ram['abilities'], self.unique)
         existing.update('tactic', self.tactic)
diff --git a/tests/objects/test_ability.py b/tests/objects/test_ability.py
index 58576b3f..65de0067 100644
--- a/tests/objects/test_ability.py
+++ b/tests/objects/test_ability.py
@@ -110,3 +110,9 @@ class TestAbility:
         test_ability.remove_all_executors()
 
         assert len(list(test_ability.executors)) == 0
+
+    def test_ability_name_duplication(self, loop, data_svc):
+        ability1 = loop.run_until_complete(data_svc.store(Ability(ability_id='12345', name='testA')))
+        ability2 = loop.run_until_complete(data_svc.store(Ability(ability_id='54321', name='testA')))
+        assert ability1.name != ability2.name
+        assert ability2.name == 'testA (2)'
diff --git a/tests/services/test_planning_svc.py b/tests/services/test_planning_svc.py
index 8055e4e0..c9184faf 100644
--- a/tests/services/test_planning_svc.py
+++ b/tests/services/test_planning_svc.py
@@ -61,8 +61,9 @@ def planner_stub(**kwargs):
 @pytest.fixture
 def setup_planning_test(loop, executor, ability, agent, operation, data_svc, event_svc, init_base_world):
     texecutor = executor(name='sh', platform='darwin', command='mkdir test', cleanup='rm -rf test')
-    tability = ability(ability_id='123', executors=[texecutor], repeatable=True, buckets=['test'])
-    tagent = agent(sleep_min=1, sleep_max=2, watchdog=0, executors=['sh'], platform='darwin', server='http://127.0.0.1:8000')
+    tability = ability(ability_id='123', executors=[texecutor], repeatable=True, buckets=['test'], name='test1')
+    tagent = agent(sleep_min=1, sleep_max=2, watchdog=0, executors=['sh'], platform='darwin',
+                   server='http://127.0.0.1:8000')
     tsource = Source(id='123', name='test', facts=[], adjustments=[])
     toperation = operation(name='test1', agents=[tagent],
                            adversary=Adversary(name='test', description='test',
@@ -71,7 +72,7 @@ def setup_planning_test(loop, executor, ability, agent, operation, data_svc, eve
                            source=tsource)
 
     cexecutor = executor(name='sh', platform='darwin', command=test_string, cleanup='whoami')
-    cability = ability(ability_id='321', executors=[cexecutor], singleton=True)
+    cability = ability(ability_id='321', executors=[cexecutor], singleton=True, name='test2')
 
     loop.run_until_complete(data_svc.store(tability))
     loop.run_until_complete(data_svc.store(cability))
diff --git a/tests/services/test_rest_svc.py b/tests/services/test_rest_svc.py
index ae4d8713..af889a66 100644
--- a/tests/services/test_rest_svc.py
+++ b/tests/services/test_rest_svc.py
@@ -22,17 +22,17 @@ def setup_rest_svc_test(loop, data_svc):
                                                 'encryption_key': 'ADMIN123',
                                                 'exfil_dir': '/tmp'})
     loop.run_until_complete(data_svc.store(
-        Ability(ability_id='123', executors=[
+        Ability(ability_id='123', name='testA', executors=[
             Executor(name='psh', platform='windows', command='curl #{app.contact.http}')
         ])
     ))
     loop.run_until_complete(data_svc.store(
-        Ability(ability_id='456', executors=[
+        Ability(ability_id='456', name='testB', executors=[
             Executor(name='sh', platform='linux', command='whoami')
         ])
     ))
     loop.run_until_complete(data_svc.store(
-        Ability(ability_id='789', executors=[
+        Ability(ability_id='789', name='testC', executors=[
             Executor(name='sh', platform='linux', command='hostname')
         ])
     ))