5 ms·
What's happening here is the 'RemoveExtraSpaces' function (ie. basically a trim) is called by the game engine every time a chat message is received. But apparen
by niftich 10y ago
What's happening here is the 'RemoveExtraSpaces' function (ie. basically a trim) is called by the game engine every time a chat message is received. But apparently, you can rebind this name and point it to any other function, including 'RunScript', which is an eval.
When this was revealed, Blizzard quickly patched in a warning dialog that warns against running scripts from untrusted sources, including being social-engineered to enter stuff yourself (which is what's happening here). That attitude seems prudent, but isn't entirely helpful, as many players in fact use Lua in their chatbox to do complex actions or calculations.
Like this one that prints your current position in 'map coordinates':
/script x,y = GetPlayerMapPosition("player");
map=GetZoneText();
c1=x*100;c2=y*100;
print(string.format("%s: %.2f, %.2f", map, c1, c2));
or the 'CTC macro' that was used to calculate a value that raid tanks must have attained:
/run DEFAULT_CHAT_FRAME:AddMessage("Need 102.4 combat table coverage. Currently at:"
..string.format("%.2f", GetDodgeChance()+GetBlockChance()+GetParryChance() +5))
It's true that users shouldn't run untrusted code, but realistically, they'll probably click through most warnings to run legitimate functions like this. A better fix is to prevent rebinding the 'RunScript' function to some other name, and to prevent rebinding the 'RemoveExtraSpaces' function by anything else.
- niftich 10y agoAnother contributing factor in this vulnerability is that the 'chat compose input' and the 'script console' are one and the same. While this was done to simplify UX, it's really not a good idea in retrospect. Even as something simple as a different chat frame that only allowed script commands, into which only local script output can go, would also serve as an effective mitigation.
- seanp2k2 10y agoOr do it like Quake and HL did and have tilde be the console. You can do things like "/say foo" or commands / settings through there, and in the case of quake at least, it can load scripts from files in a directory. This is not new stuff, and it's surprising that it doesn't do this already. Example: http://ezquake.sourceforge.net/docs/?scripting http://ezquake.sourceforge.net/docs/?scripting
- ryanlol 10y agoIt supports this already, or at least did. Console just needs to be manually enabled.
- eridius 10y agoI'm not sure how that would solve anything, the attacker would just convince the victim to type that line of code into the script console instead of the chat console.
- alistairSH 10y agoSure, the rebinding could still happen. But, the remote execution is prevented (right now, after rebinding the trim function, the attacker can enter script in the chat window to be executed - this part goes away with a separate console).
- TheCoelacanth 10y agoI don't think so. The attacker is executing script through the RemoveExtraSpaces function that gets run on every chat message, not by executing it with either player's chat console.
- niftich 10y agoThe assumption is that a proper mitigation would result in the script console being in a separate execution context; such that variables that are (re-)declared in the script console would not affect the chat console's context, or its 'RemoveExtraSpaces' function, nor the global context. I should've been more clear.
- eridius 10y agoThat assumption wouldn't work. If the script console is a separate Lua environment, it's completely useless. As long as it's the same environment that the rest of the game/addons use (which it must be), then there's no "mitigation" that can be done like you're hoping for.
- ahoy 10y agoYou (presumably) couldn't send chat messages directly to another player's script console, which is how this works.
- eridius 10y ago> A better fix is to prevent rebinding the 'RunScript' function to some other name Breaking a huge number of addons in the process, for literally no benefit because the attacker can just use that other name. > and to prevent rebinding the 'RemoveExtraSpaces' function by anything else Which wouldn't really solve anything because the attacker can just find some other function that is regularly executed with attacker-supplied input and use that instead.
- tomc1985 10y agoNo, a better fix is to remove the ability to re-bind ALL functions, or at least system calls. How is function-hotswapping-bullshit permissible in an untrusted environment?
- pdpi 10y ago> A better fix is to prevent rebinding the 'RunScript' function to some other name, and to prevent rebinding the 'RemoveExtraSpaces' function by anything else. Or maybe forbid rebinding any system-provided global symbols at all?
- eyelidlessness 10y agoOr maybe forbid rebinding any bound symbol at all.
- developer2 10y agoThis completely misses the point. Incoming chat messages from other players should simply be incapable of reaching a context in which it is interpreted as script. Inbound text chat should be treated as such - text. If the player types in commands that directly have an effect, that is their problem. If a single command provides remote code execution of anything another player types in their chat... that is simply ridiculous.
- erezyehuda 10y agoThe very existence of RunScript combined with the ability to rebind functions makes this impossible, I think. Unfortunately, I'd guess that, at this point, those features are too tangled into the game to be easily cut out.