I think I hired someone who can not code securely

ticketman · Post by **ticketman** » Mon Apr 20, 2015 11:16 am

Should I fire the person who is writing the following code for me? It is for a website that needs to be extremely secure since we will be handling credit card payments and other personal info including users' names and addresses. I do not know much about coding but from reading the sticky threads, it seems like this guy is using mysql functions and not even filtering data or using prepared statements.

------login page--------

Code: Select all

<?php include 'head.php';?>
<?php if(isset($_SESSION['tuser_id']) && $_SESSION['tuser_id'] > 0) {?>
	<script>
		window.location = 'dashboard.php';
	</script>
<?php }?>
</head>
<body class="login page page-template-default">
	<div id="wrappermain-pix" class="wrapper">
		<!-- Header Start -->
        <?php include 'header.php';?>
		<!-- Header Close -->
    	<div class="clear"></div>
    <div id="main">
		
        <!-- Inner Main -->
        <div class="innermain">
           <div class="container">
                <div class="row">
                         <div class="col-md-12">
						    <div class="element_size_100">
                        		<div class="pix-content-wrap">                    	 
                                    <header class="pix-heading-title">
                                        <h2 class="pix-section-title">Login</h2>
                                    </header>
                                    <div class="col-md-3"></div>
                                    <div class="col-md-6">
                                    <?php if(isset($_REQUEST['err']) && $_REQUEST['err'] == 1) {?>
						            	<div class="alert alert-error">
						                	<div class="inner-alert">
						                    	<span class="alert-title">ERROR:</span> Please Enter Correct Email Id and Password.
						                   	</div>
						               	</div>
						          	<?php }?>
						            <?php if(isset($_REQUEST['success']) && $_REQUEST['success'] == 1) {?>
										<div class="alert alert-success">
							                <div class="inner-alert">
							                	<span class="alert-title">Success:</span> Your Password Send In Your Email ID. Please Check and Login Again!
							               	</div>
						               	</div>
									<?php }?>
						            <div class="control-fields col-md-12" align="center">
						            	<a href=""><img src="images/fb-sign-in.png" class="FBLogin" alt="Facebook Login"/></a>
						           	</div>
						           	<div class="control-fields col-md-12" align="center">
						            	-- OR --
						           	</div>
						            <form id="commentform" action="login-proccess.php" method="post" onSubmit="return checkCheckBoxes_login();">
						            	<div class="control-fields col-md-12">
						                	<input type="text" name="username" required="" class="col-md-12" placeholder="Email Address" id="username" />
						              	</div>
						                <div class="control-fields col-md-12">
						                	<input type="password" name="password" required="" class="col-md-12" id="password" placeholder="Password" />
						                </div>
						                <div class="control-fields col-md-12" align="center">
						                	<input type="submit" name="login-process" id="submit" class="btn btn-custom btn-red btn-medium btn-large-text" value="Log in with Email" />
						               	</div>
						                <div class="control-fields col-md-12" align="center"><a href="forgot-password.php">Forgot Password ?</a> &nbsp;|&nbsp;  <a href="registration.php">Registration</a></div>
						         	</form>
						         	</div>
						         	<div class="col-md-3"></div>
                                    </div>
                                </div>
                    		</div>
						    		
         				</div>
					</div>
	            </div> 
	        </div>
		</div>
	<?php include 'footer.php';?>
	</div>
	<?php include 'footer-js.php';?>

------script that processes the login page--------

Code: Select all

<?php
//Start session
	session_start();
	
	include 'superadmin/connect.php';
	if(isset($_REQUEST['login-process'])) {
		$username = $_REQUEST['username'];
		$pass = $_REQUEST['password'];
		
		$qry = "SELECT * FROM `user` WHERE `email` = '$username' AND `password` = '".(md5($pass)."'";
		$result = mysql_query($qry);
		//echo $qry;die;

		//Check whether the query was successful or not
		if($result) {
			if(mysql_num_rows($result) == 1) {
				//Login Successful
				session_regenerate_id();
				$member = mysql_fetch_assoc($result);
				$_SESSION['tuser_id'] = $member['id'];
				$_SESSION['tuser_email'] = $member['email'];
				$_SESSION['tuser_fname'] = $member['first_name'];
				$_SESSION['tuser_lname'] = $member['last_name'];
				$_SESSION['tuser_postal'] = $member['post_code'];
				$_SESSION['tlast_login'] = gmdate('dS F Y h:i');

				ob_end_flush();
				session_write_close();
				$actios = '';
				?>
					<script type="text/javascript">
						window.location = 'dashboard.php';
					</script>
				<?php  
				} else {
				//Login failed
				?>	
				<script type="text/javascript">
					window.location = 'login.php?err=1';
				</script>
			<?php
					exit();
				}
			} else {
				//Login failed
		?>	
				<script type="text/javascript">
					window.location = 'login.php?err=1';
				</script>
		<?php
				exit();
			}
	}
	?>	
	<script type="text/javascript">
		window.location = 'login.php?err=1';
	</script>
<?php exit();?>

Post by **pickle** » Mon Apr 20, 2015 12:32 pm

Once I saw

Code: Select all

$username = $_REQUEST['username'];
$pass = $_REQUEST['password'];

$qry = "SELECT * FROM `user` WHERE `email` = '$username' AND `password` = '".(md5($pass)."'";

I stopped reading. This is horrible. There is no sanitation of user provided input, so it would be incredible easy to compromise your database. On top of that, the password is only hashed with MD5, which hasn't been reliable as a password hashing mechanism for years.

User input sanitation is quite possible the first and most important aspect of web development security. If the person didn't get this right, I personally wouldn't trust them to do anything properly.

Post by **Benjamin** » Mon Apr 20, 2015 8:12 pm

Yeah, as pickle said, this is bad.

If you don't have the budget for a good project manager, much less the developers he'd want to employ, you don't have the budget to be PCI compliant either.

So you'll just need to throw some real money at this or look into the authorize.net vault api. But probably both if you want to see success.

Firing this guy and hiring another in the same price range would result in the same type of code. There's no craftsmanship here.

ticketman · Post by **ticketman** » Tue Apr 21, 2015 4:56 pm

What would you consider "real money" when it comes to an ecommerce website? This guy was charging be about 5k but now I fired him and hired someone else for about the same amount.

Post by **Benjamin** » Fri Apr 24, 2015 7:30 pm

Depends on what complexity. $5k for a database driven site is on the low end but it can be done. The thing is, if the person doesn't do it right you can end up in a situation where the site fails when you need it the most. e.g. you get a rush of orders and it goes down. Things like that can put you out of business quickly.

A senior developer would want about $100k a year.

You'd also need a designer, because senior developers typically aren't that good at art & design.

Depending on traffic, you might also want a DBA, which could be another 100k a year.

I'm sure others will chime in here. The numbers are all over the place. The hardest part is finding true talent.

PHP Developers Network

I think I hired someone who can not code securely

I think I hired someone who can not code securely

Re: I think I hired someone who can not code securely

Re: I think I hired someone who can not code securely

Re: I think I hired someone who can not code securely

Re: I think I hired someone who can not code securely