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 missing wrapping in t_handler() #110

Merged
merged 1 commit into from
May 9, 2022
Merged

fix missing wrapping in t_handler() #110

merged 1 commit into from
May 9, 2022

Conversation

pyvain
Copy link

@pyvain pyvain commented Apr 16, 2022

This is a proposition to fix #108.
The fix proposed in #109 corrects the issue in most cases without properly identifying the cause, which might cause other issues.

The issue

When we deal with concatenation expressions in evaluate_parse_tree(), the retrieved value is always unwrapped (for example at lines 2274 and 2276).

However in t_handler(), the returned EvalResult does not wrap the value.

This is ok when the value does not start and end with quotes (this is the majority of cases), since unwrap_str_literal() does nothing if the string to unwrap does not start and end with quotes.

But if the result of t_handler() starts and ends with quotes, they are removed in evaluate_parse_tree(), and we get a wrong value to evaluate the rest of the formula, with missing quotes.

As a reference, here is the definition of the T() excel function:
The Excel T function returns text when given a text value and an empty string ("") for numbers, dates, and the logical values TRUE and FALSE. You can use the T function to remove values that are not text.

The fix

The fix I'm proposing consists in always wrapping the result of t_handler() since it is text by definition. That way we do not loose quotes when unwrapping.

@DissectMalware DissectMalware merged commit 4b3d371 into DissectMalware:master May 9, 2022
@DissectMalware
Copy link
Owner

thank you for the PR

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.

Unexpected Token
2 participants