Skip to content

Conversation

@tagyieh
Copy link

@tagyieh tagyieh commented Feb 4, 2025

Analyzer Draft

This PR is a work in progress to implement an analyzer for the existing MAL compiler, described in this issue. It is a continuation of the previous analyzer draft, which can be found here.

Implementation

malVisitor in mal_visitor.py will use an analyzer malAnalyzerInterface. The visit method is overridden in malVisitor in order to call the checker-methods in the malAnalyzerInterface (if defined).

Analyzer Implementation

Language Constituents

Constituent Implemented Tested Reviewed
Define
Include
Category
Asset
Extends
Abstract asset
User info
Developer info
Modeler info
Let
Association
Field
Step

MAL Symbols

Constituent Implemented Tested Reviewed
Leads to (->)
Append (+>)
OR-step (|)
AND-step (&)
Defense (#)
Existence (E)
Non-existence (!E)
Require (<-)
TypeOf ([])
Union (\/)
Intersection (\/)
Difference (-)
Collect Operator (X.A)
Transitive Operator (X*.A)
Let Substitution (X().A)
Hidden Step (@​hidden) - -
Debug Step (@​debug) - -
Trace Origin (@​trace) - -
Risk Type (AS {C, I, A})
Time to Compromise (TTC)

Notes/Possible checks

  • Can an Asset with the same name be defined twice?
  • Can we use a same include more then once? (Risk of circular dependency) Can "id" or "version" be defined more than once/with different values?
  • Should the state of the analyzer be synchronous between all files when using include?
  • Should we allow two detectors on the same asset with the same name?
  • Should we allow an empty detector context?
  • Should we allow assets in the detector context that do not exist?
  • Should we a class that extends another belong to the same category as the parent?
  • Can a category be defined more than once?
  • Given the documentation, the only allowed metas are 'developer', 'users' and 'modelers'. Should we check if the metas fall under one of these three categories?

WLUB and others added 26 commits February 12, 2024 18:54
Deleted support for union, intersection and difference. These are operations usdo to create a set of assets, meaning that they could never appear in a reaches, unless in an expr. Therefore, this check_to_step should only take care of the case collect, where it will return the asset pointed by the lhs (where the set operation can be) and checks if the rhs is indeed a step for the specified asset.

Deleted support for subType. This is used to denote a subclass, so in the context of an attack step can only be used inside a collect, where the lhs is the subType and the rhs is the associated step.

Added comments.
Previously, checking for field and variable was done in the same place. To keep the code manageable and similar to its Java counterpart, this was split into separate functions.

Added logging message in case the user mentioned an existing variable but forgot to call it.

Added comments.
Previously, variable was not considered a case in _check_to_asset, but it should be.

Added comments.
This concludes the verification of all subfunctions as well.
Added verification to ensure repeated defines does not happen.

Added comments.
Instead of verifying if an error was found and then setting the class error as True, set it as soon as an error is found.

Added comments.
Deleted a raise instruction which wasn't raising anything.

Added comments.
The previous implementation, when given a step, would verify if this step was defined by another asset. However, according to the Java check, we should verify the step hierarchy, i.e. if a step is properly extended from the assets parents. For this, when analysing a single step we only check if it is already defined in the asset. In the post analysis, we verify if the steps are properly inherited.

Added comments.
@nkakouros nkakouros mentioned this pull request Feb 4, 2025
30 tasks
@nkakouros
Copy link
Contributor

Can an Asset with the same name be defined twice?

Isn't there a check in the java analyzer? I think in MAL it does not make sense to allow the same labels to be used twice, either in associations, assets or let-variables.

Can we use a same include more then once? (Risk of circular dependency)

Probably yes, we want to support this. Circular dependencies may be possible to capture and report to the user as error.

Can "id" or "version" be defined more than once/with different values?

Isn't there a check in the java analyzer? In this case, I think yes, as that would make possible to extend a whole language (that has its own version and id).

Should the state of the analyzer be synchronous between all files when using include?

What do you mean here? I think the analyzer should keep track of assets, etc across all files (starting from main.mal) so that it can properly check things. Also, check my changes on the visitor class, moving it to __init__.py and the loading of files here. Compare it with what is in the main branch and your branch. It would be nice if you could extract the relevant bits from there into a separate commit that you push here. The detector stuff, disregard for the time being.

Should we allow two detectors on the same asset with the same name?
Should we allow an empty detector context?
Should we allow assets in the detector context that do not exist?

Don't care about detectors for the time being.

Should we a class that extends another belong to the same category as the parent?

@andrewbwm Do you have any input here?

Can a category be defined more than once?

I think yes, and we should merge. @andrewbwm

Given the documentation, the only allowed metas are 'developer', 'users' and 'modelers'. Should we check if the metas fall under one of these three categories?

Probably not, there are lots of custom metas that we don't want to break.

@nkakouros
Copy link
Contributor

@tagyieh Resolve the conflicts too.

Also changed test verification. If a test fails, it is not relevant to know what components of the language are stored in the analyzer; we should only verify that to guarantee that the analyzer correctly processes everything in case of success.
Since we allow for multiple metadata (not only user, modeler and developer), it does not make sense to have specific tests for them.
Tests in operation and in transitive are temporarily altered to ensure all tests pass.
This funciton shall log the error and raise the exception.
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.

3 participants