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

Enhance with statement to allow variable reuse within the same statement #290

Closed
wants to merge 1 commit into from

Conversation

r0ny123
Copy link

@r0ny123 r0ny123 commented Jan 16, 2025

No description provided.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi team,

I'm currently reviewing pull request #[PR number] by r0ny123 and will provide my feedback shortly. In the meantime, here's a summary to get everyone up to speed:

This pull request enhances the with statement in YARA to allow variable reuse within the same statement. This improves the readability and conciseness of YARA rules.

Intent:
The goal is to make YARA rules easier to write and understand by allowing variables defined in a with statement to be used in subsequent variable declarations within that same with statement. This reduces the need for nested with statements.

Changes:

  • The documentation (docs/Module Developer's Guide.md) has been updated to reflect the new functionality with examples demonstrating basic usage, nested with statements, and the ability to use variables in other declarations within the same with statement (lines 1109-1174).
  • Unit tests have been added to lib/src/compiler/ir/tests/mod.rs (lines 138-159) to verify the correct implementation of the new feature.
  • The compiler (lib/src/compiler/mod.rs) has been modified to support variable reuse within a single with statement (lines 1244-1247). Specifically, a FxHashMap called with_vars has been added to track variables within the scope of a with statement.

Summary Table:

File Change Lines Changed
docs/Module Developer's Guide.md Added section explaining the enhanced with statement functionality with multiple examples. 66
lib/src/compiler/ir/tests/mod.rs Added a new test case (with_statement_variable_usage) to verify the correct implementation of the new feature. 22
lib/src/compiler/mod.rs Added with_vars to the Compiler struct to manage variables within the with statement. 1

I'll be back with a more detailed review soon.

And now, a little haiku to brighten your day:

Code flows, clean and bright,
With statements, now more clear,
YARA shines anew.

@r0ny123 r0ny123 closed this Jan 16, 2025
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request enhances the with statement in YARA, allowing variables declared within the statement to be reused in subsequent variable declarations within the same statement. This is a valuable improvement that promotes cleaner and more readable YARA rules by reducing the need for nested with statements or redundant calculations. The provided documentation and test cases clearly illustrate the new functionality. Overall, the changes are well-implemented and contribute positively to YARA's usability.

Comment on lines +1122 to +1132
rule example {
condition:
with
a = 1 + 1,
b = a + 2:
(
b == 4
)
}
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

The YARA code example can be improved by assigning more meaningful values to the variables. This will make the example easier to understand and demonstrate the purpose of variable reuse more effectively.

Suggested change
```yara
rule example {
condition:
with
a = 1 + 1,
b = a + 2:
(
b == 4
)
}
```
rule example {
condition:
with
initial_value = 10,
offset = initial_value + 5,
size = offset * 2 : (
size == 30
)
}

Comment on lines +1159 to +1170
```yara
rule example {
condition:
with
a = 1 + 1,
b = a + 2,
c = b + 3:
(
c == 7
)
}
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Similar to the previous comment, using more realistic values in this example will enhance its clarity and demonstrate the practical application of the feature more effectively.

Suggested change
```yara
rule example {
condition:
with
a = 1 + 1,
b = a + 2,
c = b + 3:
(
c == 7
)
}
```
rule example {
condition:
with
base_address = 0x400000,
entry_point_rva = 0x1000,
entry_point = base_address + entry_point_rva:
(
entry_point == 0x401000
)
}

Comment on lines +144 to +154
let source = r#"
rule test {
condition:
with
call = @code[0],
rtn_ptr = call + !code[0] + 4:
(
rtn_ptr == 0
)
}
"#;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

This test case can be strengthened by adding more complex expressions within the with statement, including nested expressions and multiple variable dependencies. This will ensure that the functionality is thoroughly tested under various scenarios.

Suggested change
let source = r#"
rule test {
condition:
with
call = @code[0],
rtn_ptr = call + !code[0] + 4:
(
rtn_ptr == 0
)
}
"#;
let source = r#"
rule test {
condition:
with
base = 0x400000,
rva = pe.entry_point_raw,
entry_point = base + rva,
another_offset = entry_point + 0x1000: (
entry_point > 0 and another_offset < filesize
)
}
"#;

@@ -1244,6 +1244,7 @@ impl Compiler<'_> {
vars: VarStack::new(),
for_of_depth: 0,
features: &self.features,
with_vars: FxHashMap::default(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add a with_vars field to the CompileContext to store variables declared within a with statement. This field should be an FxHashMap where keys are variable names and values are their corresponding ExprIds.

with_vars: FxHashMap::default(),

@r0ny123 r0ny123 deleted the enhance-with-statement branch January 16, 2025 06:19
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