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.

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

Fix entry point id lookup in initial actions collection #82

Closed
wants to merge 1 commit into from

Conversation

nkakouros
Copy link
Collaborator

@nkakouros nkakouros commented Nov 29, 2024

When an attack graph gets constructed, nodes are added to it with IDs that are sequential and map exactly to the nodes' indices in the graph's node list. (In case we are loading from a file, the node IDs are used as described in the file; but this makes no difference to the rest of the discussion).

In the simulator, when an attack graph gets loaded, pruning of nonviable and unnecessary nodes may happen:

apriori.calculate_viability_and_necessity(attack_graph)
if prune_unviable_unnecessary:
apriori.prune_unviable_and_unnecessary_nodes(attack_graph)

(BTW, wouldn't it be more fitting if prune_unviable_unnecessary) was a sim setting?)

This will result in indices in attack_graph.nodes not mapping to the node.ids anymore. That's fine, we have the MalSimulator._id_to_index and _index_to_id properties to follow the nodes in the list.

The issue here, when collecting the initial actions, is that there is no reason to go from the node.id to the list index. If truncation is in place this will result in the wrong node being picked up. There is no way for the user to control this, at least if they start with a handcrafted model file, or one generated via Securicad (and potentially the KTH UI tool), together with a MAL file; node IDs do not appear in any of those.

I believe there is a broader issue in the simulator when truncation happens, i.e. there may be more places where the translation between indices and node IDs is either missing or messing. For example a few lines below my edit, for the defender.

@nkakouros
Copy link
Collaborator Author

False alert, all seems good here.

@nkakouros nkakouros closed this Nov 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant