Skip to content

Commit

Permalink
Fix incorrect load-time errors for non-hotkey lines containing '::'
Browse files Browse the repository at this point in the history
  • Loading branch information
@Lexikos
Lexikos committed Apr 20, 2024
1 parent c51d218 commit b1ac375
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
9 changes: 6 additions & 3 deletions source/hotkey.cpp
View file
Expand Up @@ -1833,10 +1833,13 @@ ResultType Hotkey::TextToKey(LPCTSTR aText, bool aIsModifier, Hotkey *aThisHotke
else
{
// If there's something after the first word/character, it's not a hotkey.
if (aSyntaxCheckOnly)
return *keyname_end ? FAIL : OK;
// Only for *keyname_end != '\0', no error should be reported if aThisHotkey == NULL
// since the caller will attempt to parse the line as something else first, such as
// x := '::'
if (*keyname_end)
return ValueError(ERR_INVALID_HOTKEY, aText, FAIL);
return aThisHotkey ? ValueError(ERR_INVALID_HOTKEY, aText, FAIL) : CONDITION_FALSE;
if (aSyntaxCheckOnly)
return OK;
}

HotkeyTypeType hotkey_type_temp;
Expand Down
2 changes: 1 addition & 1 deletion source/script.cpp
View file
Expand Up @@ -2843,7 +2843,7 @@ bool Script::IsSOLContExpr(LineBuffer &next_buf)
// MsgBox
// +!'::' ; 0
*hotkey_flag = '\0';
bool valid_hotkey = Hotkey::TextInterpret(next_buf, NULL, true);
bool valid_hotkey = Hotkey::TextInterpret(next_buf, NULL, true) == OK;
*hotkey_flag = *HOTKEY_FLAG;
if (!valid_hotkey)
return true; // It's not valid hotkey syntax, so treat it as continuation even if it's ultimately a syntax error.
Expand Down

6 comments on commit b1ac375

@rubahness
Copy link

@rubahness rubahness commented on b1ac375 Apr 23, 2024 •
edited

Choose a reason for hiding this comment

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

The windows setup executable from 2.0.13 gets flagged as a virus by Windows Security. The Windows setup executable from 2.0.12 does not.
image

@topia
Copy link

@topia topia commented on b1ac375 Apr 23, 2024

Choose a reason for hiding this comment

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

You should use the forum for an issue. https://www.autohotkey.com/boards/viewtopic.php?f=82&t=129146

@Lexikos
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rubahness That is an issue to take up with Microsoft. It is a false positive, but even if the file was infected, it would have nothing to do with this commit.

@rubahness
Copy link

Choose a reason for hiding this comment

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

@topia thank you for the link to the forums. I didn't see one in this project, which is why I created this issue.

@Lexikos this seemed to be the final commit before the PR was completed for .13. the issue didn't occur with the executable from .12, therefore it seemed reasonable to infer whatever change is triggering windows security occurred in-between those releases.

Maybe it's a Microsoft problem, but as a rando, I don't know that. The XZ exploit is still recent news. Sorry to have wasted the 30 seconds of your time

@Lexikos
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rubahness There is no logic to signature or machine learning (AI) based detection. When the file was initially flagged, likely before any other human had seen it, the detection name indicated it was based on machine learning (AI). I reported the false positive to Microsoft. The response, which I was never actually notified of (but happened to see because I checked my submission), was that they (probably more AI) have updated their signatures to detect the file as malware (name in your screenshot). They gave no explanation and provide no means of disputing the determination, aside from submitting another report, which I did. Again I received no notification for a response, and now the links for my submissions do not work. If I seemed short with you in my previous comment, maybe now you see why. Nothing personal.

The setup exe is a compiled script based on AutoHotkey32.exe and AutoHotkey/AutoHotkeyUX@42fb651c. It includes a payload of files built from the AutoHotkey, AutoHotkeyUX and AutoHotkey-Docs repositories. This process was automated by Lexikos/AutoHotkey-Release@701af1f (mainly here). None of those files were flagged last I checked. Literally all of the code that makes up the setup.exe is in those files, except for whatever UPX does. (The file was compressed with UPX, which can also decompress it. I haven't bothered to test the decompressed file against any scanners.)

I had also built a setup exe based on the parent commit of this one and the parent commit of AutoHotkey/AutoHotkeyUX@42fb651c, and that exe was not flagged. There is no rational explanation for why these two commits would cause a detection, aside from random chance.

which is why I created this issue

This is a comment on a commit, not an issue in an issue tracker. Personally I think the natural conclusion upon failing to find an issue tracker would be that issues should be raised elsewhere. The project URL is AutoHotkey's website, the front page of which has a very obvious link to the forums. But given that many others have posted misplaced issues (before I disabled the issue tracker) or comments on commits, if I don't put prominent directions for support in the readme, it's my own fault.

@joedf
Copy link
Member

@joedf joedf commented on b1ac375 Apr 29, 2024

Choose a reason for hiding this comment

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

@Lexikos You could add/copy what I have below to the readme, whatever makes sense to you.

Support

  • Bug Reporting: Whenever possible, pull requests are preferred. Otherwise, please report any potential bugs or issues you find here.
  • False positives: If you notice any AutoHotkey files (downloaded from official sources) are being flagged as suspicious or a virus, they are likely false positives. Please refer to our page on how to resolve or report these here.
  • Other development topics: For any other development related inquires, please discuss them here

Please sign in to comment.