My app is constantly being hacked. Please help!

[sonnieboy]

		public	function AddFiles($name = 'item')
			{
				// If the files array has been set
				if(isset($_FILES[$name]['name']) && !empty($_FILES[$name]['name'])) {

						// Remove empties
						$_FILES[$name]['name']      =   array_filter($_FILES[$name]['name']);
						$_FILES[$name]['type']      =   array_filter($_FILES[$name]['type']);
						$_FILES[$name]['size']      =   array_filter($_FILES[$name]['size']);
						$_FILES[$name]['tmp_name']  =   array_filter($_FILES[$name]['tmp_name']);

						// we need to differentiate our type array names
						$use_name   =   ($name == 'item')? 'Addend':$name;
						// To start at Addendum1, create an $a value of 1

						$a = 1;
						if(!empty($_FILES[$name]['tmp_name'])) {

								 foreach($_FILES[$name]['name'] as $i => $value ) {
										$file_name  =   ms_escape_string($_FILES[$name]['name'][$i]);
										$file_size  =   $_FILES[$name]['size'][$i];
										$file_tmp   =   $_FILES[$name]['tmp_name'][$i];
										$file_type  =   $_FILES[$name]['type'][$i];

										if(move_uploaded_file($_FILES[$name]['tmp_name'][$i], $this->target.$file_name)) {
												// Format the key values for addendum
												if($name == 'item')
													$arr[$use_name.$a]      =   $file_name;
												// Format the key values for others
												else
													$arr[$use_name] =   $file_name;

												$sql	=   $this->FilterRequest($arr);

												// Auto increment the $a value
												$a++;
											}
									}
							}
					}

				if(isset($sql) && (isset($i) && $i == (count($_FILES[$name]['tmp_name'])-1)))
					$this->data[$name]	=	$sql;

				return $this;
			}

		public	function SaveFolder($target = '../uploads/')
			{
				$this->target	=	$target;

				// Makes the folder if not already made.
				if(!is_dir($this->target))
					mkdir($this->target,0755,true);

				return $this;
			}

		public	function where($array = array())
			{
				$this->where_vals	=	NULL;

				if(is_array($array) && !empty($array)) {
						foreach($array as $key => $value) {
								$this->where_vals[]	=	$key." = '".ms_escape_string($value)."'";
							}
					}

				return $this;
			}

		public	function UpdateQuery()
			{
				$this->data			=	array_filter($this->data);

				if(empty($this->data)) {
						$this->statement	=	false;
						return $this;
					}

				if(isset($this->data) && !empty($this->data)) {

						foreach($this->data as $name => $arr) {
								$update[] =   implode(",",$arr['update']);
							}
					}

				$vars		=	(isset($update) && is_array($update))? implode(",",$update):"";

				// Check that both columns and values are set
				$this->statement	=	(isset($update) && !empty($update))? "update bids set ".implode(",",$update):false;

				if(isset($this->where_vals) && !empty($this->where_vals)) {
						$this->statement	.=	" where ".implode(" and ",$this->where_vals);
					}

				return $this;
			}

		public	function SelectQuery($select = "*",$table = 'bids')
			{
				$stmt				=	(is_array($select) && !empty($select))? implode(",",$select):$select;

				$this->statement	=	"select ".$stmt." from ".$table;

				return $this;
			}

		public	function InsertQuery($table = 'bids')
			{
				$this->data			=	array_filter($this->data);

				if(empty($this->data)) {
						$this->statement	=	false;
						return $this;
					}

				$this->statement	=	"insert into ".$table;

				if(isset($this->data) && !empty($this->data)) {

						foreach($this->data as $name => $arr) {
								$insert['cols'][] =   implode(",",$arr['cols']);
								$insert['vals'][] =   implode(",",$arr['vals']);
							}
					}

				$this->statement	.=	'(';
				$this->statement	.=	(isset($insert['cols']) && is_array($insert['cols']))? implode(",",$insert['cols']):"";
				$this->statement	.=	") VALUES (";
				$this->statement	.=	(isset($insert['vals']) && is_array($insert['vals']))? implode(",",$insert['vals']):"";
				$this->statement	.=	")";

				return $this;
			}
	}

Greetings experts,

 

I have an app that hackers are constantly hacking by inserting junk data and uploading files to our server.

 

I think this code below is the source of the hacking.

 

There is a whole lot more code but I think this is the relevant code.

 

I will be more than happy to post more.

 

Any assistance in helping me shore this up to stand the test of hacking will be greatly appreciated.

 

 

[mac_gyver]

what user based security do you have in the application to control who the form processing code is accessible to?

 

Edit: also, it appears that your sql queries have NO protection against sql injection, so it's possible that some of the junk you are seeing is the after math of attempts to inject sql, which can be used to dump all the contents of any of your database tables out to the whoever submitted the data.

Edited March 7, 2018 by mac_gyver

[ginerjm]

I don't see any comments in this code pointing at whatever you suspect is the problem. That's odd. What makes you think this is the block of code with the problem? Have you examined the actual file that is stored on the server that is actually being executed? That's where I would be looking. For one thing - your server file s/b different than the source code on your client where it was written. You should be doing a line-by-line compare if you think the problem is in this file.

 

Of course if as suggested by others you are being hacked by your poorly written queries, then you have more concerns than just this script. If I were you I'd be changing all my sensitive passwords since they are probably not safe. Of course the hacker could get a fresh copy of them pretty fast so it's only temporary.

[sonnieboy]

Of course if as suggested by others you are being hacked by your poorly written queries

I just love it when some people think it makes them feel more important by putting others down.

[ginerjm]

I don't understand your comment. Mac_gyver pointed out the weakness in your code wherein malicious users can do a lot of damage. You think he said that to make himself feel better than you?

 

Ok - I'm done. Not only do you not know how to solve this problem but you don't know how to relate to people doing exactly what you asked for.

[requinix]

You aren't restricting what sorts of files can be uploaded so someone could upload PHP scripts if they wanted. Depending on your setup, they may even be able to upload to any directory on your website or server. And uploads will overwrite each other.

[sonnieboy]

		public	function AddFiles($name = 'item')
			{
				// If the files array has been set
				if(isset($_FILES[$name]['name']) && !empty($_FILES[$name]['name'])) {

						// Remove empties
						$_FILES[$name]['name']      =   array_filter($_FILES[$name]['name']);
						$_FILES[$name]['type']      =   array_filter($_FILES[$name]['type']);
						$_FILES[$name]['size']      =   array_filter($_FILES[$name]['size']);
						$_FILES[$name]['tmp_name']  =   array_filter($_FILES[$name]['tmp_name']);

						// we need to differentiate our type array names
						$use_name   =   ($name == 'item')? 'Addend':$name;
						// To start at Addendum1, create an $a value of 1

						$a = 1;
						if(!empty($_FILES[$name]['tmp_name'])) {

								 foreach($_FILES[$name]['name'] as $i => $value ) {
										$file_name  =   ms_escape_string($_FILES[$name]['name'][$i]);
										$file_size  =   $_FILES[$name]['size'][$i];
										$file_tmp   =   $_FILES[$name]['tmp_name'][$i];
										$file_type  =   $_FILES[$name]['type'][$i];

										if(move_uploaded_file($_FILES[$name]['tmp_name'][$i], $this->target.$file_name)) {
												// Format the key values for addendum
												if($name == 'item')
													$arr[$use_name.$a]      =   $file_name;
												// Format the key values for others
												else
													$arr[$use_name] =   $file_name;

												$sql	=   $this->FilterRequest($arr);

												// Auto increment the $a value
												$a++;
											}
									}
							}
					}

				if(isset($sql) && (isset($i) && $i == (count($_FILES[$name]['tmp_name'])-1)))
					$this->data[$name]	=	$sql;

				return $this;
			}

First of all,why did you bring mac_gyver into this?

 

My commented was intended for YOU,  not him.

 

Read his comments, read requinix comments. They are pointing out what they thought are issues with my app getting hacked. They are pointing me in the right direction.

 

I don't have problems with those constructive criticisms. You remind me of a guy I used to work with who has a tendency act like he knows everything and everybody else knows nothing but never offers real solutions to issues.

 

Anyway, requinix, if you don't mind, is this the code where I need to restrict file types that can be uploaded?

 

You suggestion makes a lot of sense. Thinking about it now, it may not necessarily mean that the app is being hacked into. It may be that they are just uploading all kinds of file types because no restrictions are in place.

[requinix]

Anyway, requinix, if you don't mind, is this the code where I need to restrict file types that can be uploaded?

That code does handle uploads, but to present an error message to the user you'll probably have to check earlier in the process.

 

Not that you can't do both: check the file at a point where you can present information back to the user, and at the point where it processes the upload.

 

You suggestion makes a lot of sense. Thinking about it now, it may not necessarily mean that the app is being hacked into. It may be that they are just uploading all kinds of file types because no restrictions are in place.

Look in the upload directory for files that shouldn't be there. That's the easiest exploit.

 

Then check the rest of your site for files that shouldn't exist, or that have a "recent" modification time.

[ginerjm]

You think I didn't give you something constructive? How about this from my original post:

For one thing - your server file s/b different than the source code on your client where it was written. You should be doing a line-by-line compare if you think the problem is in this file.

Anyone worth their salt would take this approach immediately if they suspect a specific file just to SEE if the script was altered. That's how many hacks begin.

 

I am so sorry you missed my constructive idea because you were in such a hurry to scold me. Too bad.

[requinix]

Okay, everybody's had a chance to make their argument about what being constructive is and is not, let's move on from that.

[dalecosp]

Some general advice: read through the server logs and find where this script is being accessed. If it does it work by using the POST method, look only for POST actions to this URL.

 

You might also wish to add some logging statements to the questionable code using something like "file_put_contents($some_log_message, '/tmp/mylogs');", so that you can read '/tmp/mylogs' later and see what's being done to your script. Example code:

 

$variable = filter_input(INPUT_POST, 'foo');
file_put_contents("received $variable from POST", '/tmp/mylogs', FILE_APPEND);

Next, when something is POSTed, check that the refering page is the one you expect it to be. If it isn't, reject the input. Also investigate the possibility of placing a token in the form that will vary from pageload to pageload and must be validated by the POST routine on the receiving script. (This is basically a CSRF protection).

 

Finally, consider adding Google's ReCaptcha to any forms that are being abused. Although we'd all like to think that some Russian Mafioso is sitting at his computer targeting our site personally, the truth is that most attacks are by "bots" and the ReCaptcha technology is pretty good at weeding those out.

Edited March 9, 2018 by dalecosp