sylvanaar
commented
5 years ago Did it work in retail?
Closed quixadhal closed 4 years ago
sylvanaar
commented
5 years ago Did it work in retail?
quixadhal
commented
5 years ago I don’t remember having any issues with it in retail last year, but I haven’t played retail since just after BFA launched. I don’t have retail installed, or I’d test it for you.
Sent from Mail for Windows 10
From: Jon S Akhtar Sent: Saturday, August 31, 2019 8:34 AM To: sylvanaar/prat-3-0 Cc: Dread Quixadhal; Author Subject: Re: [sylvanaar/prat-3-0] API changes for Classic? (#7)
Did it work in retail? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.
Mitalie
commented
5 years ago Are you using tab completion? Blizzard's tab completion has a chance to break as soon as any addon registers a custom slash command. The tab completion goes through all registered slash commands in the internal order of the table holding them. If it comes across a custom one before finding one matching what you typed, the text contents of the editbox become tainted and you can't use secure slash commands without emptying the editbox first.
quixadhal
commented
5 years ago I’ll check this. I assume there’s an option or module to disable this entirely? I don’t consciously know I’m using it, but 30+ years of programming in bash and tcsh makes tab completion somewhat of an automatic habit.
Since lua tables are much like python’s dictionaries and perl’s hash tables, I’ll also assume that once ANY custom things are added to a table, they can affect this outcome since order is random but consistent based on the hash algorithm used internally.
If the issue is that any lua function inserting text into the editbox causes it to be tainted, then this would also affect command history as well?
I’ll try to remember to check both, after I play the 45 minute login queue mini-game. 😊
Thanks guys!
Sent from Mail for Windows 10
From: Anssi Mäkinen Sent: Sunday, September 1, 2019 1:50 PM To: sylvanaar/prat-3-0 Cc: Dread Quixadhal; Author Subject: Re: [sylvanaar/prat-3-0] API changes for Classic? (#7)
Are you using tab completion? Blizzard's tab completion has a chance to break as soon as any addon registers a custom slash command. The tab completion goes through all registered slash commands in the internal order of the table holding them. If it comes across a custom one before finding one matching what you typed, the text contents of the editbox become tainted and you can't use secure slash commands without emptying the editbox first. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.
quixadhal
commented
5 years ago Just a quick update, this does indeed seem to be the case.
If I type /targetfriend and hit return, it will work correctly. If I hit return, up-arrow, return, which would pull that command from history, it fails. Apparently ANY editing of user input that's done outside of the stock blizzard keybindings causes the data to become tainted. I'm 99.9% sure that doesn't happen in retail, or at least didn't back when I played it. I know it was pretty common for me to use the history to reissue commands with different targets, to save typing.
Mitalie
commented
5 years ago Tab completion is a default UI feature, and it doesn't come with an explicit disable option. However you can define ChatEdit_CustomTabPressed() to return a truthy value, and the default completion function won't be called.
Yes, hash table order. Macro oneliner /run if IsAltKeyDown() then t=nil end t=next(hash_ChatTypeInfoList,t) print(t,issecurevariable(hash_ChatTypeInfoList,t)) can be used to investigate. Safest assumption is that as soon as you load an addon that defines a slash command, tab completion will taint the editbox content. Some commands may work in some configurations though, since order may depend on size of the table (affecting hash truncation) as well as insertion order (for hash collisions, depending on how the implementation handles them).
Command history is a feature of the EditBox widget itself, implemented in C, so using it doesn't automatically taint the content. History entries (editBox:AddHistoryLine()) preserve taint state just like the current content (editBox:SetText()) does, restoring it via editBox:GetText() when you modify or execute them, but you can navigate past tainted entries and execute secure commands from untainted lines.
Depending on your usage command history may indeed be what's spreading the taint. Taint can be picked up by executing a custom command, a default command that touches something tainted further down the call chain, or even a completely clean but non-secure command if ChatFrame_ImportAllListsToHash() processes a custom command that was added to SlashCmdList since previous import. The execution path may then lead to a editBox:SetText() or editBox:AddHistoryLine(), storing the taint in the editbox. If you then go back and change it into a secure command without completely emptying the editbox, the command will fail.
Upon further testing and code-reading I realized that using up/down arrows without holding alt key is a Prat feature in the Editbox module added in response to a Blizzard change back in 2013. It integrates with Prat's History module but being non-blizzard code will taint the editbox contents. To use the built-in secure history, you must hold alt key while pressing up or down arrows.
Perhaps Prat's History module could be changed to not save secure commands because they will error when executed. On the other hand, remembering secure commands may still be useful, especially if saving across sessions which default UI doesn't do, and the commands can "cleaned" by the user by copying the text into clipboard and pasting it back into the editbox.
sylvanaar
commented
5 years ago Arrow up/down is one of my most used features. Thank you for the analysis.
The editbox used to be written in such a way that even if the text was tainted it would clear the taint and everything would work. It had to be that way if you think about it. Being able to taint slash commands and the chatframe as a whole is fundamental to its operation, and being able to mod it at all.
There have always been "issues" that I wanted to fix about both tab-complete and command history, I guess it is really time to dig in here.
thefowles1
commented
5 years ago Chiming in since I was directed to this thread - it does in fact happen in retail, both tab completion and up-arrow re-entry from history. I don't recall for how long this has happened to me, but it has been a long, long time (many months to possibly over a year - to respond to @quixadhal being unsure if it happens in retail).
quixadhal
commented
5 years ago I should mention that the only reason I noticed it was that I was trying to debug a macro in classic, and thus trying various combinations of modifiers like [noharm] and similar. A major PITA to have to manually retype or constantly copy/paste the lines.
I had no idea there was an alt-arrow-key thing that performed the same (basic) function. So that's a workaround if I can force myself to remember it, thanks! :)
I wonder if it would be possible to detect the taint status of the previous line when hitting up-arrow, and disable completion (or anything else that might taint the input) if the line being restored was untainted? Hmmm.... likely to be sub-optimal no matter how you look at it I guess.
sylvanaar
commented
4 years ago This can be fixed by going to the editbox module and choosing the option to use the ALT key for movement. Also, the history module - disable saving of history between sessions.
Not sure if it is possible to clear the taint. Closing for now
Not sure if there's anything that can be done for this, but this prevents you from executing commands like "/targetenemy" when using Prat in Classic WoW.