Authors CurseForge

This globalstring from 4.3.4 client makes me wonder...

CONFIRM_LAUNCH_URL = "Clicking \"Okay\" will take you out of the game and open a web browser.";

Edit: The plot thickens :P

StaticPopupDialogs["CONFIRM_LAUNCH_URL"] = {
    text = CONFIRM_LAUNCH_URL,
    button1 = OKAY,
    button2 = CANCEL,
    OnAccept = function(self, data) LoadURLIndex(data); end,
    hideOnEscape = 1,
    timeout = 0,
}

I wonder if LoadURLIndex(data) can be called from addons with arbitrary URLs or index refers exclusively to one of

VISITABLE_URL1 = "http://enUS.nydus.battle.net/WOW/enUS/client/in_game_mail_link_1?r=US&l=%s&sr=%u&c=%x%x";
VISITABLE_URL2 = "http://enUS.nydus.battle.net/WOW/enUS/client/trial_restriction_error_1?region=US&accountName=%s";
VISITABLE_URL3 = "http://enus.nydus.battle.net/wow/enUS/client/item-restoration";
VISITABLE_URL_GENERIC = "http://enUS.nydus.battle.net/WOW/enUS/client/in_game_mail_link_%d";

Edit2: Posted this here as it might be related to the "Addons cannot access anything outside of WoW" stipulation if these changes allow us to send a url to the browser.
Feel free to delete or move it if it proves irrelevant.

Quote from Ketho

I just tested it on Live and it indeed seems it exclusively accepts the indices for the GlueStrings you mentioned

Yep:

% grep -r LoadURLIndex .
./FrameXML/ItemRef.lua:         LoadURLIndex(tonumber(index));
./FrameXML/StaticPopup.lua:     OnAccept = function(self, data) LoadURLIndex(data); end,
./GlueXML/GlueParent.lua:       LoadURLIndex(2);
%

The code in ItemRef.lua is for a new clickable link of type "urlIndex" (like "item", "channel", "player", etc, are now). The code in GlueParent.lua is for a function called UpgradeAccount(). It looks very much like these are meant to be Blizzard-only, but I'm surprised the function wasn't simply restricted in that case. If the implementation maps a number to a VISITABLE_URL global, then addons could just replace that string and off they go. The obvious assumption is that the implementation is then checking the contents of those globals against a known safe list, and the globalstrings are just for easy localization and display purposes. That's my working theory, anyhow. :-)

Quote from Farmbuyer

If the implementation maps a number to a VISITABLE_URL global, then addons could just replace that string and off they go

Exactly :p (I was already picturing direct dkp exports in my head as I was posting this, just skipped the obvious parts)

Thanks Dridzt. This is interesting.

LoadURLIndex() exists since Patch 4.2.0 (June 2011).

syntax:
LoadURLIndex(index)
index must be a number otherwise this produce an error

I did not find a way to open a custom website. All LoadURLIndex calls open a webpage from Blizzard.
And Blizzard is well-advised to never change this.

But I think the current behaviour is dangerous, very dangerous.
An addon can simply add LoadURLIndex(0) to the first line of an addon and the default webbrowser opens with a battle.net address. No check or confirm is required for that action. Blizzard should immediatly hotfix this behaviour.
Sure, it's only the browser and it's only a Blizzard website, but it's very dangerous in terms of security.

I think it's plain stupid to allow WoW execute an executable (eg: firefox.exe) from within WoW with a single line of code in an addon. Damned plain stupid!!!
Who is checking the integrity of this file? Warden? ...

Blizzard should simply delete LoadURLIndex(). now. hotfix now!


Thanks, Dridzt for bringing this up.

Well it looks like Farmbuyer was correct anyway.

I had a chance to test in-game, replacing the VISITABLE_URLx globals and calling LoadURLIndex(x) with a valid index still opens the blizzard addresses so these are obviously pulled from the binaries (or mpqs) and not the lua environment.

I mean

/run _G["VISITABLE_URL3"] = "http://www.google.com";LoadURLIndex(3)

still opens the blizzard address.

I don't see this as such a huge security risk, it opens the system browser.
It's not an addon risk as anyone with a compromised web browser executable would be already vulnerable from simply browsing the net or opening a link from any application, having an addon open the browser is simply "pointless" in that situation.

Quote from kunda

I think it's plain stupid to allow WoW execute an executable (eg: firefox.exe)

WoW doesn't say what browser to run. There's no pathname or executable file.

WoW tells the OS "open the preferred browser, whatever it is, and handle this URL". What happens then is outside its control, the same as with any other program opening any other URL.

The point is: With LoadURLIndex() it's possible to compromise (hack/steal) a WoW account with one simple thing: an addon.

Some addons, even here at wowace.com comes with executables (mainly *.bat files). It's easy with a batch file to change the registry key that defines the default browser. On windows systems this is the key HKEY_CLASSES_ROOT\[protocol]\shell\open\command ([protocol] = http, https, ftp or so). Most WoW players use some Windows OS. To make such changes with a batch file you normally need special rights. The experienced user will never run such file, but I do not talk about experienced users, I am focused on the average user. The average user simply downloads something and runs something to achieve something. Simply cover such actions behind a wall of well-written, well-formed grammar, add some nice screenshots and garnished it with a little kitten or a brutal warrior and...bingo - (or worse: use your wowace-established author name to make a little update on an existing addon that use a batch file, manipulate it and pretend that you just made a superduper update...your credibility is gone but you can harm people...open repositories...even one victim is more than enough...) This kind of users are the main target for any acc hacker. Such users have simply a lack of knowledge and therefor such users may run such batch file, even if they are forced to run it with privileged rights. Pretend to explain why they must run this batch file... You think this can/does not happen? Wake up.

Can you follow? If you can bring a user to run such batch file, this user is doomed. If this user runs WoW the addon calls LoadURLIndex(0) and the default browser is no longer a browser, it's some other code/prog/whatever [imagination on]. Because LoadURLIndex(0) does not require a confirm dialog the user has no chance to avoid the execution of potential compromised code/progs/...

Sure, this will not work with any user. Sure, you need some imagination, but 1% is enough, even 1 is enough!!!

If user/account security has any priority for Blizzard they must disable LoadURLIndex(). The absolute minimum is to disable it for addons and a confirm dialog.

Quote from kunda

The point is: With LoadURLIndex() it's possible to compromise (hack/steal) a WoW account with one simple thing: an addon.

No. This has nothing to do with LoadURLIndex. If you can convince your users to run .bat files, then you can already do things outside of WoW's scope. At that point, you don't need to run WoW, or even run a web browser. If I've convinced you to run a third-party batch file containing arbitrary commands, then I don't need to trick an addon into running a compromised web browser!

Also, it's utterly pointless to argue about running a web browser when the set of allowable URLs is fixed. The mapping from index numbers to URL strings is hardcoded into the game client; the VISITABLE_URL* strings are an exported copy of those mappings. They are "write-only" and are never read back in by the API.

Plenty of programs fire off web browsers. The possibility that the browser is compromised is not the responsibility of those programs.

working example of addon code which can cause an actual problem:

LoadURLIndex(0)

Okay, because no one is interested in all that destructive hack fuss, let's talk about some positive applications. Let's start doing some real-time one-way meta-addon. The combination of Screenshot() and LoadURLIndex(0) can be used to transport ingame information in real-time (black background - white text - Screenshot() - start some ocr prg with LoadURLIndex(0) [yes this requires a modified default browser key] - do something with that data). ... This offers unimaginable opportunities.