Security concerns.

Support and feedback for UniAdmin

Security concerns.

Postby Brimstone » Tue Feb 27, 2007 11:27 pm

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.
Brimstone
WR.net Apprentice
WR.net Apprentice
 
Posts: 10
Joined: Tue Feb 27, 2007 10:53 pm

Security concerns.

Postby Fangorn » Wed Feb 28, 2007 12:33 am

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...
Last edited by Fangorn on Wed Feb 28, 2007 12:35 am, edited 1 time in total.
ImageImageImage
User avatar
Fangorn
WR.net Apprentice
WR.net Apprentice
 
Posts: 35
Joined: Mon Jul 24, 2006 4:26 am
Location: Ashburn, Virginia, USA

Re: Security concerns.

Postby MattM » Wed Feb 28, 2007 12:45 am

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.
MattM
UA/UU Developer
UA/UU Developer
Gimpy Developer
Gimpy Developer
 
Posts: 886
Joined: Tue Jul 04, 2006 9:53 pm
Location: USA

Re: Security concerns.

Postby Brimstone » Wed Feb 28, 2007 1:19 am

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.
Brimstone
WR.net Apprentice
WR.net Apprentice
 
Posts: 10
Joined: Tue Feb 27, 2007 10:53 pm

Re: Security concerns.

Postby Brimstone » Wed Feb 28, 2007 1:25 am

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)
Brimstone
WR.net Apprentice
WR.net Apprentice
 
Posts: 10
Joined: Tue Feb 27, 2007 10:53 pm

Security concerns.

Postby zanix » Wed Feb 28, 2007 3:27 am

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?
Read the Forum Rules, the WiKi, and Search before posting!
WoWRoster v2.1 - SigGen v0.3.3.523 - WoWRosterDF
User avatar
zanix
Admin
Admin
WoWRoster.net Dev Team
WoWRoster.net Dev Team
UA/UU Developer
UA/UU Developer
 
Posts: 5546
Joined: Mon Jul 03, 2006 8:29 am
Location: Idaho Falls, Idaho
Realm: Doomhammer (PvE) - US

Security concerns.

Postby MattM » Wed Feb 28, 2007 4:03 am

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?
Last edited by MattM on Wed Feb 28, 2007 4:05 am, edited 2 times in total.
MattM
UA/UU Developer
UA/UU Developer
Gimpy Developer
Gimpy Developer
 
Posts: 886
Joined: Tue Jul 04, 2006 9:53 pm
Location: USA

Re: Security concerns.

Postby MattM » Wed Feb 28, 2007 4:06 am

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.
MattM
UA/UU Developer
UA/UU Developer
Gimpy Developer
Gimpy Developer
 
Posts: 886
Joined: Tue Jul 04, 2006 9:53 pm
Location: USA

Re: Security concerns.

Postby sweede » Wed Feb 28, 2007 8:09 am

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
WR.net Apprentice
WR.net Apprentice
 
Posts: 5
Joined: Sun Feb 18, 2007 6:21 pm

Re: Security concerns.

Postby Brimstone » Wed Feb 28, 2007 12:54 pm

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?
Last edited by Brimstone on Wed Feb 28, 2007 12:58 pm, edited 1 time in total.
Brimstone
WR.net Apprentice
WR.net Apprentice
 
Posts: 10
Joined: Tue Feb 27, 2007 10:53 pm

Security concerns.

Postby Fangorn » Wed Feb 28, 2007 6:02 pm

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
Last edited by Fangorn on Wed Feb 28, 2007 7:20 pm, edited 4 times in total.
ImageImageImage
User avatar
Fangorn
WR.net Apprentice
WR.net Apprentice
 
Posts: 35
Joined: Mon Jul 24, 2006 4:26 am
Location: Ashburn, Virginia, USA

Security concerns.

Postby MattM » Wed Feb 28, 2007 7:40 pm

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.
MattM
UA/UU Developer
UA/UU Developer
Gimpy Developer
Gimpy Developer
 
Posts: 886
Joined: Tue Jul 04, 2006 9:53 pm
Location: USA

Re: Security concerns.

Postby Brimstone » Wed Feb 28, 2007 7:58 pm

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.
Brimstone
WR.net Apprentice
WR.net Apprentice
 
Posts: 10
Joined: Tue Feb 27, 2007 10:53 pm

Re: Security concerns.

Postby Brimstone » Wed Feb 28, 2007 8:07 pm

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.)
Brimstone
WR.net Apprentice
WR.net Apprentice
 
Posts: 10
Joined: Tue Feb 27, 2007 10:53 pm

Re: Security concerns.

Postby Brimstone » Wed Feb 28, 2007 8:10 pm

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.)
Last edited by Brimstone on Wed Feb 28, 2007 8:15 pm, edited 2 times in total.
Brimstone
WR.net Apprentice
WR.net Apprentice
 
Posts: 10
Joined: Tue Feb 27, 2007 10:53 pm

Next

Return to UniAdmin

Who is online

Users browsing this forum: No registered users and 1 guest

cron