In a lot of the threads I see suggestions for setting permissions 777 everywhere, and even suggested patches for putting this inside the code of the application for saved addons.

This would however make it so that EVERY other user on the system could remove/replace your addons. This also opens up the possibility for the installation of loggers, etc. (There is a bug in the current UniUploader that has it extract files to the WoW directory itself. Such that you could easily make a zip file that would overwrite WoW.exe or Launcher.exe.) If you think this would never happen, then you're nieve. Guilds are already having their sites hacked and all their items sold off by gold farmers.

Read here about a guild that got raped and lost most their epics on account of lax website security, with no restoration from Bliz.:
http://wow-en.curse-gaming.com/other/70 ... storation/
http://wow-en.curse-gaming.com/general- ... y-loggers/


As such, I really think you should revise your handling of permissions to suggest 0700 in all cases.

I see your concern here... If there is a vulnerability in uniadmin that allows to access the upload interface, or if an admin's account is hacked, then someone could upload a malevolent addon that would be pushed to all guildmembers.

Your solution would imply changing the access rights every time an addon needs to be uploaded/updated to temporarily grant the upload rights. That's a bit of a pain if you want several people to manage the addons (or even if it's only you :p).


An easy fix, limit access to the uniadmin interface by IP address at the webserver level. For apache, create a .htaccess file in the uniadmin directory that contains the following:

Code: Select all

<Files index.php>
Order Deny,Allow
Deny From all
Allow From [list of IPs of the uniadmin admins]
</Files>

Only works if you have a static IP of course...



However. When you write "This would however make it so that EVERY other user on the system could remove/replace your addons", this is system security, not software security. If you install uniadmin on a shared system where outsiders can modify your file structure, that's your own problem really...

Brimstone wrote:(There is a bug in the current UniUploader that has it extract files to the WoW directory itself. Such that you could easily make a zip file that would overwrite WoW.exe or Launcher.exe.)

Negative, observe the source code for the addon unzipper uu uses:

Code: Select all

if (extention != ".exe" && extention != ".com" && extention != ".jar" && extention != ".vbs" && extention != ".bat")
               {
                  DebugLine("Writing File: " + directoryName+@""+fileName);
                  FileStream streamWriter = File.Create(directoryName+@""+fileName);

As you can see I have thought of this particular securety concern.

But all of this means you have to compile your own binary using the source code, after you review the source code, in order to ensure it isn't malicious.

About UA having securety holes:
Did you find one? tell us about it and we will fix it, thats what open source non profit is all about, the community contribution.

What I mean is more of a documentation and user education issue.

I'm also worried about things like users on large shared hosting sites where there might be hundreds of guilds. File permissions in some of your docs indicate 0777, which leads to vulnerabilities.

The locking down of remote IP for the admin interface is a good one, and I might implement it. But more so I wanted to make sure you didn't allow for random users with shell access to overwrite items in the addons_zips folder.

So, for example, this thread has a user doing the evil 777 permissions:
http://www.wowroster.net/Forums/viewtopic/t=2727.html

This thread has a proposed change to make all extracted files world-writable:
http://www.wowroster.net/Forums/viewtop ... rt=15.html

Code: Select all

according to the pclzip library you can add the option to set the permission of the directory there,
$list = $archive->extract(PCLZIP_OPT_PATH, $path, PCLZIP_OPT_SET_CHMOD, 0777);


As does your readme.txt on the uniadmin download:

Code: Select all

2 - Installation
================
1. Create a new database (eg. uniadmin)
2. Upload the contents of the zip file to your webserver
3. After FTPing, CHMOD the following folders to 0777, or change NTFS file permissions for these folders to make them
   available as "Everyone - Write" on a Windows machine.


In fact, only Apache needs to be able to write to the files.

I'd even go so far as to think that ideally, uniadmin should check for world-writable directories within its structure and refuse to operate if there are any, much the same way many mail servers won't read your settings files if they are world-writable.

I realize that most of these threads are not from uniadmin's authors, but rather the user base. But I think theuser base needs education and I think the tool should assume a userbase that is uninformed and/or negligent about security issues.

MattM wrote:Negative, observe the source code for the addon unzipper uu uses:

Code: Select all

if (extention != ".exe" && extention != ".com" && extention != ".jar" && extention != ".vbs" && extention != ".bat")
               {
                  DebugLine("Writing File: " + directoryName+@""+fileName);
                  FileStream streamWriter = File.Create(directoryName+@""+fileName);

As you can see I have thought of this particular securety concern.

OK, this is definitely a big step. It's not mentioned in the docs, but I'm glad that it is in there. Though there's a lot more malicious file types then just those three I'm afraid. Even .scr is evil these days. So maybe reverse it, maintain a list of known 'safe' file types and only extract those? (lua, toc, txt, tga, blp)

I think I need to revise UA to not scan these file extensions as well

So, if I ask the user to set the directories to 700, and if they set the cmod from say.. a ftp client, will UA still be able to write to those directories?

you can also make it so only the temp_addon folder needs to be world writable, by storing the binary data in the database, PM me if you want the code to convert binary to hex and back.

The changes needed for this type of change aren't severe.

edit: also is there a way to have pclzip read a binary string instead of a file?

Brimstone wrote:

MattM wrote:Negative, observe the source code for the addon unzipper uu uses:

Code: Select all

if (extention != ".exe" && extention != ".com" && extention != ".jar" && extention != ".vbs" && extention != ".bat")
               {
                  DebugLine("Writing File: " + directoryName+@""+fileName);
                  FileStream streamWriter = File.Create(directoryName+@""+fileName);

As you can see I have thought of this particular securety concern.

OK, this is definitely a big step. It's not mentioned in the docs, but I'm glad that it is in there. Though there's a lot more malicious file types then just those three I'm afraid. Even .scr is evil these days. So maybe reverse it, maintain a list of known 'safe' file types and only extract those? (lua, toc, txt, tga, blp)

very good idea, I think I'll do that.

Brimstone wrote: In fact, only Apache needs to be able to write to the files.

You are correct, Apache needs to be able to write the files.

Unix permissions are octal codes there are three types, read write execute.

read is 4, write is 2, execute is 1.

then there are 3 groups,
User, Group and World.


unfourtuntly, apache does not run as your user or your group on most systems. Apache will run as its own user (usually http).

when apache is running as its own user, which will be totally seperate from your own (group and user permissions), you will need to give "world" access to the directory and files that apache needs to modify.

this is why you need 777.

sweede wrote:

Brimstone wrote: In fact, only Apache needs to be able to write to the files.

You are correct, Apache needs to be able to write the files.

Unix permissions are octal codes there are three types, read write execute.
...
this is why you need 777.

I laughed, I cried...

You are -so- wrong that I can't help by laugh myself silly.

If you hosting service or whatever has told you this, move to a company that cares about your security, cuz this one don't.

'What you need' as you put it is that the file be writable, not any specific permissions. 0777 always guarantees this, but it also always guarantees that you are insecure. Less permissions work in many proper setups.

Allow me to express myself in Perl, it should be easy for everyone to read:

Code: Select all

#Assume needed use statements.
my $stat = stat('/dir/where/I/am/installed');
if ($UID == $stat->uid) {
  # Permissions should be 0700/0600;
  die if $stat->mode & 0700;
} elsif ($GID == $stat->gid) {
  # Permissions should be 0770/0660;
  die if $stat->mode & 0770;
} else {
  warn "You have a crap ISP, move!";
}


As for your belief that Apache means all files have to be world writable, I suggest you either use a box you own (like I do) where you can just chown everything to apache directly, or get a reputible ISP that uses Apache's suexec features. (Dreamhost for example.)

http://wiki.dreamhost.com/index.php/Suexec
http://faq.siteground.com/faq/file-permissions.html

ISPs, by and large, do not like having thousands of their customers taken over because they all followed some instructions to chmod everything in sight to 777. It's a very expensive cleanup when a thousand websites suddenly start hosting phishing pages or keyloggers.

Oh, and if you somehow misunderstood my initial post to imply that I am not versed in Unix security or Apache, I should let you know I was on the SSH2 IETF task force and have been a Unix user and sysadmin for about 15 years now. I'm currently responsible for writing software to secure what are literally some of the largest data centers in existence, upwards of 8,000 *nix boxes at a single site.

So, please, no attempts at remedial lessons for me, OK?

Brimstone,

PHP usually isn't running as a CGI so suexec doesn't apply here. PHP will run as the webserver's user and will require the web user to have write access to the directory.

If your host knows what he's doing, your directory will be sealed from other users directories. This ensures no other hosted account has the right to read/modify your files using a PHP or CGI program (regardless of the chmod because the upper directory structure/open_basedir restrictions prevent it).

If you're hosting your own box and do whatever it is you want to do then of course feel free to chown your directory structure to the apache's user and limit the chmod to 700.

Now if you have a better solution than what the industry currently does to handle access rights when hosting several hundred thousand users you should start your own hosting company because what you seem to have in mind but don't express very clearly is so revolutionary you're going to make billions

I laughed, I cried...

You are -so- wrong that I can't help by laugh myself silly.

Otherwise I'd recommend showing a bit of humility and learning more on multi website hosting before dissing the people on these boards that invest time to develop this software and answer your questions


- Fang

guys lets not start a flame war, not that it is, just saying.

that said, lets find a way to unzip an addon zip into binary strings while it is in binary string zip file form.

if we can accomplish that there will BE no need for special folder permissions.

Fangorn wrote:Brimstone,
If your host knows what he's doing, your directory will be sealed from other users directories. This ensures no other hosted account has the right to read/modify your files using a PHP or CGI program (regardless of the chmod because the upper directory structure/open_basedir restrictions prevent it).

Care to cite references? Or to explain to me how one 'seals' a directory other than using file permissions? If Apache isn't running suexec, it certainly isn't running chrooted, and you'd have to have a separate apache parent per home dir.

While you may not be able to see the other user's files while you're logged in via FTP, they are certainly still there. FTPd's have historically had a lot more options to chroot post-connect. Apache doesn't.

So, please, by all means, show me some links to docs that prove me wrong, or teach me of this 'sealing' you speak of.

MattM wrote:guys lets not start a flame war, not that it is, just saying.

that said, lets find a way to unzip an addon zip into binary strings while it is in binary string zip file form.

if we can accomplish that there will BE no need for special folder permissions.

Agreed. If this degrades any further into flamage, then I'll just go away. I don't have time for drama.

The following comes form the manual for your zip library:

Code: Select all

This method extract all or part of the files contained in the PKZIP file.
Filtering can be done by the optional arguments PCLZIP_OPT_BY_NAME, PCLZIP_OPT_BY_EREG, PCLZIP_OPT_BY_PREG and PCLZIP_OPT_BY_INDEX.
...
PCLZIP_OPT_EXTRACT_AS_STRING gives the ability to extract the content of a file in the returned array rather than writing it in a file. This can be usefull for example for extracting the readme file of an archived package, before doing the full extract.


As such, you can first get the list of names from the file, and then get the data back as strings. You can then store them directly. It seems you can extract them all at once, or individually, depending on if you want to extract on the files one at a time (probably by index) or do them all at once. (Optimization for memory vs. runtime.)

MattM wrote:very good idea, I think I'll do that.

Also following the design principle of 'least surprise', I would suggest that you add an else clause that does something meaningful. Otherwise there will be confusion when a specific file is not extracted. Either a popup error of some kind, "Addon file contain unknown/unsafe file types: @list_of_files", or touching instead empty files that indicate what files were skipped, 'Interface/Addon/FooAddOn/Upload.exe not extracted for security'

Additionally, there is this:

Code: Select all

or remove only a prefix of this path (PCLZIP_OPT_REMOVE_PATH).


So if you were to universally remove the 'Interface/AddOn' part of the path, then that would solve some of the problems you've been having with zips that don't include those, as all zip files would come out at just one directly level. (Unless this is already addressed.)