From 17b0edf952e878ae0f90a3eed9fb5f7e83a8c364 Mon Sep 17 00:00:00 2001 From: QianEric Date: Thu, 5 Nov 2026 18:43:33 +0800 Subject: [PATCH] olevba: don't describe _Click handlers as running on document open AUTOEXEC_KEYWORDS_REGEX puts every ActiveX/control event in one group described as "Runs when the file is opened and ActiveX objects trigger events", including \w+_Click. That description is wrong for _Click: CommandButton1_Click fires when the user clicks the control and at no other time, so a document whose only macro entry point is a button was reported as having code that runs on open. Some of the other events in the group can plausibly fire while a control is initialised or rendered during load (_GotFocus, _LostFocus, _Resize, _Layout), so the wording is not wrong for them. Only _Click is split out. Detection is unchanged - \w+_Click still matches and is still reported as AutoExec, as introduced in #499. Only the description differs. The exact wording comes from the issue's suggestion; happy to reword. Test: tests/olevba/test_autoexec.py calls detect_autoexec directly and checks that CommandButton1_Click gets the interaction description (and that it contains no "file is opened" marker), that TextBox1_GotFocus and Workbook_Open keep their on-open descriptions, and that a document containing both a click and a GotFocus handler reports the two separately. Two of the five cases fail on master, all five pass with the change; tests/olevba/ as a whole gives 11 passed, 1 skipped. Note the maintainer may want to look at _Change and _FollowHyperlink in the same group, which are arguably interaction-only too - I left them alone to keep this change to what the issue asked for. --- oletools/olevba.py | 7 +++- tests/olevba/test_autoexec.py | 76 +++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 tests/olevba/test_autoexec.py diff --git a/oletools/olevba.py b/oletools/olevba.py index 23be15b7a..15cd5d956 100644 --- a/oletools/olevba.py +++ b/oletools/olevba.py @@ -640,8 +640,13 @@ def __init__(self, stream_path, variable, expected, value): 'Runs when the file is opened (using InkPicture ActiveX object)': # ref:https://twitter.com/joe4security/status/770691099988025345 (r'\w+_Painted', r'\w+_Painting'), + # Interaction-only events. A \w+_Click handler fires when the user clicks an + # embedded control, never on document load, so it must not be described as + # running when the file is opened. Detection is unchanged (added in #499). + 'Runs when the user interacts with an embedded ActiveX control (e.g. clicks a button)': + (r'\w+_Click',), 'Runs when the file is opened and ActiveX objects trigger events': - (r'\w+_GotFocus', r'\w+_LostFocus', r'\w+_MouseHover', r'\w+_Click', + (r'\w+_GotFocus', r'\w+_LostFocus', r'\w+_MouseHover', r'\w+_Change', r'\w+_Resize', r'\w+_BeforeNavigate2', r'\w+_BeforeScriptExecute', r'\w+_DocumentComplete', r'\w+_DownloadBegin', r'\w+_DownloadComplete', r'\w+_FileDownload', r'\w+_NavigateComplete2', r'\w+_NavigateError', diff --git a/tests/olevba/test_autoexec.py b/tests/olevba/test_autoexec.py new file mode 100644 index 000000000..cf714d19d --- /dev/null +++ b/tests/olevba/test_autoexec.py @@ -0,0 +1,76 @@ +""" +Tests for detect_autoexec keyword classification in olevba. + +Covers the interaction-only control events that were previously described as +"Runs when the file is opened ..." (see issue #891): detection must not change, +only the description, so that downstream consumers do not read a user-clicked +button as an auto-executing entry point. +""" + +import unittest + +from oletools.olevba import detect_autoexec + + +# Descriptions that assert the code runs when the document is opened. Used to +# check that interaction-only events are not reported with one of these. +ON_OPEN_MARKERS = ('file is opened', 'document is opened', 'Workbook is opened') + +CLICK_DESCRIPTION = ( + 'Runs when the user interacts with an embedded ActiveX control ' + '(e.g. clicks a button)' +) + + +class TestAutoexecInteractionEvents(unittest.TestCase): + """detect_autoexec must describe user-interaction events accurately""" + + def test_click_event_is_not_described_as_on_open(self): + """A _Click handler is described as interaction, not as on-open.""" + code = 'Private Sub CommandButton1_Click()\nEnd Sub\n' + results = detect_autoexec(code) + + self.assertEqual(len(results), 1, msg='expected exactly one match: %r' % (results, )) + keyword, description = results[0] + self.assertEqual(keyword, 'CommandButton1_Click') + self.assertEqual(description, CLICK_DESCRIPTION) + for marker in ON_OPEN_MARKERS: + self.assertNotIn(marker, description) + + def test_click_event_is_still_detected(self): + """Splitting the description must not drop the detection from #499.""" + code = 'Sub CommandButton1_Click()\nEnd Sub' + keywords = [keyword for keyword, _desc in detect_autoexec(code)] + self.assertIn('CommandButton1_Click', keywords) + + def test_gotfocus_event_keeps_the_on_open_description(self): + """Other control events still use the on-open description.""" + code = 'Sub TextBox1_GotFocus()\nEnd Sub' + results = detect_autoexec(code) + self.assertEqual(len(results), 1, msg='expected exactly one match: %r' % (results, )) + _keyword, description = results[0] + self.assertEqual( + description, 'Runs when the file is opened and ActiveX objects trigger events' + ) + + def test_workbook_open_keeps_the_on_open_description(self): + """Plain workbook-open handlers are unaffected by this change.""" + code = 'Private Sub Workbook_Open()\nEnd Sub' + results = detect_autoexec(code) + self.assertEqual(len(results), 1, msg='expected exactly one match: %r' % (results, )) + _keyword, description = results[0] + self.assertEqual(description, 'Runs when the Excel Workbook is opened') + + def test_click_and_gotfocus_are_reported_separately(self): + """Both events are reported, each with its own description.""" + code = 'Sub CommandButton1_Click()\nEnd Sub\nSub TextBox1_GotFocus()\nEnd Sub' + results = dict(detect_autoexec(code)) + self.assertEqual(results.get('CommandButton1_Click'), CLICK_DESCRIPTION) + self.assertEqual( + results.get('TextBox1_GotFocus'), + 'Runs when the file is opened and ActiveX objects trigger events', + ) + + +if __name__ == '__main__': + unittest.main()