Mysql sikkerhet..

sindredit · 4 Jul 2008

Hei alle! Er vel første posten min på forumet

Har laget en publiseringsløsning som jeg nå driver å konverterer til MySQL, har liten erfaring med MySQL så håper at noen av dere kan sjekke om denne koden er sikker eller ikke

PHP:

<?php
session_start();
ini_set('arg_separator.output', '&amp;');
/**
*
* File: adm/inc/new_user.php
* Version 19:56 26.06.2008
* Copyright (c) 2008 Sindre Andre Ditlefsen
* License: http://opensource.org/licenses/gpl-license.php GNU Public License
*
*/

if($control_panel == "$_SESSION[user_id]" && $new_user == "$_SESSION[user_id]"){
	if($action == "add"){
		$inp_forename = $_POST['inp_forename'];
		$inp_forename = quote_smart($inp_forename);
		if(empty($inp_forename)){
			$error = "<p><b>Du må fylle inn feltet «Fornavn» før brukeren kan lagres.</p>";
			$action = "";
		}

		$inp_surname = $_POST['inp_surname'];
		$inp_surname = quote_smart($inp_surname); 
		if(empty($inp_surname)){
			$error = "<p><b>Du må fylle inn feltet «Etternavn» før brukeren kan lagres.</p>";
			$action = "";
		}
		
		$inp_email = $_POST['inp_email'];
		$inp_email = quote_smart($inp_email);
		if(empty($inp_email)){
			$error = "<p><b>Du må fylle inn feltet «E-post» før brukeren kan lagres.</p>";
			$action = "";
		}

		$inp_password = $_POST['inp_password'];
		$inp_password = quote_smart($inp_password);
		if(empty($inp_password)){
			$error = "<p><b>Du må fylle inn feltet «Passord» før brukeren kan lagres.</p>";
			$action = "";
		}


		if($error == ""){
			// Connect to database
			$inp_password_saying = $inp_password;
			$inp_password = md5("$inp_password");
mysql_query("INSERT INTO users_basic 
(id, forename, surname, email, password) VALUES('', $inp_forename, $inp_surname, $inp_email, '$inp_password') ") 
or die(mysql_error());
			echo"
			<h1>Bruker opprettet</h1>

			<p>Informasjon:</p>
		<table width=\"100%\">
		 <tr>
		  <td class=\"outline\">
			<table width=\"100%\" cellspacing=\"1\">
			"; if($style == "bodycell"){$style = "subcell";} else{ $style = "bodycell";} echo"
			 <tr>
			  <td class=\"$style\" valign=\"top\">
		<span><b>Fornavn</b></span>
			  </td>
			  <td class=\"$style\" valign=\"top\">
		<span>$inp_forename</span>
			  </td>
			 </tr>
			"; if($style == "bodycell"){$style = "subcell";} else{ $style = "bodycell";} echo"
			 <tr>
			  <td class=\"$style\" valign=\"top\">
		<span><b>Etternavn</b></span>
			  </td>
			  <td class=\"$style\" valign=\"top\">
		<span>$inp_surname</span>
			  </td>
			 </tr>
			"; if($style == "bodycell"){$style = "subcell";} else{ $style = "bodycell";} echo"
			 <tr>
			  <td class=\"$style\" valign=\"top\">
		<span><b>E-post</b></span>
			  </td>
			  <td class=\"$style\" valign=\"top\">
		<span>$inp_email</span>
			  </td>
			 </tr>
			"; if($style == "bodycell"){$style = "subcell";} else{ $style = "bodycell";} echo"
			 <tr>
			  <td class=\"$style\" valign=\"top\">
		<span><b>Ønsket passord</b></span>
			  </td>
			  <td class=\"$style\" valign=\"top\">
		<span>$inp_password_saying</span>
			  </td>
			 </tr>
			</table>

		  </td>
		 </tr>
		</table>
			<p><a href=\"index.php?p=users\">Tilbake</a></p>
			";
		}
	}
	if($action == ""){


		echo"
		<h1>Ny bruker</h1>
		
		<form action=\"?p=new_user&amp;action=add\" method=\"post\" enctype=\"multipart/form-data\">
		$error
				
		<table width=\"100%\">
		 <tr>
		  <td class=\"outline\">
			<table width=\"100%\" cellspacing=\"1\">
			"; if($style == "bodycell"){$style = "subcell";} else{ $style = "bodycell";} echo"
			 <tr>
			  <td class=\"$style\" valign=\"top\">
		<span><b>Fornavn</b></span>
			  </td>
			  <td class=\"$style\" valign=\"top\">
		<span><input type=\"text\" name=\"inp_forename\" value=\"$inp_forename\" size=\"49\" /></span> <span style=\"color:red;\">*</span>
			  </td>
			 </tr>
			"; if($style == "bodycell"){$style = "subcell";} else{ $style = "bodycell";} echo"
			 <tr>
			  <td class=\"$style\" valign=\"top\">
		<span><b>Etternavn</b></span>
			  </td>
			  <td class=\"$style\" valign=\"top\">
		<span><input type=\"text\" name=\"inp_surname\" value=\"$inp_surname\" size=\"49\" /></span> <span style=\"color:red;\">*</span>
			  </td>
			 </tr>
			"; if($style == "bodycell"){$style = "subcell";} else{ $style = "bodycell";} echo"
			 <tr>
			  <td class=\"$style\" valign=\"top\">
		<span><b>E-post</b></span>
			  </td>
			  <td class=\"$style\" valign=\"top\">
		<span><input type=\"text\" name=\"inp_email\" value=\"$inp_email\" size=\"49\" /></span> <span style=\"color:red;\">*</span>
			  </td>
			 </tr>
			"; if($style == "bodycell"){$style = "subcell";} else{ $style = "bodycell";} echo"
			 <tr>
			  <td class=\"$style\" valign=\"top\">
		<span><b>Ønsket passord</b></span>
			  </td>
			  <td class=\"$style\" valign=\"top\">
		<span><input type=\"text\" name=\"inp_password\" value=\"$inp_password\" size=\"49\" /></span> <span style=\"color:red;\">*</span>
			  </td>
			 </tr>
			</table>

		  </td>
		 </tr>
		</table>
		<p><input type=\"submit\" value=\"Lagre\" /></p>

		<p><a href=\"index.php?p=users\">Tilbake til brukere</a></p>
		</form>
		";
	}
}
else{
	echo"<h1>Tilgang nektet</h1>

	<p>Beklager, brukeren din har ikke rettigheter til å benytte seg av denne modulen. Vær vennlig
	å kontakt sidens administrator for å få tilgang.</p>

	<p><a href=\"index.php?p=manual#403\">Hjelp</a></p>";
}
?>

Gomlers · 5 Jul 2008

Beklager at det her blir off topic, men håper det er ok for det!
Jeg er ingen rev på mysql, men jeg er en rev på å se når noe er bra. Og jeg må si at det her ser %%"# ta meg helt genialt ut!!

Flott design, tøft adminpanel, enkelt og greit.. og flott video!!

Dette skal prøves Sindre!!

Selger du dette eller?

btw. minor detail.. du sa i filmen jeg så at du ville vi skulle rapportere skrivefeil: Installere skrives med bare en n

Men ellers helt fantastisk!

tyr897 · 5 Jul 2008

Når det gjelder sikkerheten ser ser SQL-biten grei ut, men du er sårbar mot XSS. Du skriver ut brukerinput direkte, <span>$inp_forename</span>, og man kan da injisere f.eks. javascript kode. Bruk htmlentities e.l. når du skriver ut brukerinput du ikke har filtrert.
Tenk godt igjennom hvilke data som faktisk hører hjemme, og bruk whitelisting under validering. Du kan f.eks. begrense brukernavn til a-z0-9_-, da har du samtidig beskyttet deg mot XSS for den variabelen. Valider at e-post-adressen faktisk kunne vært nettopp det, osv.

Merk at du ikke har filtrert input i databasen, slik at når du henter data derfra, må du passe på å bruke en escape funksjon dersom du skal skrive den ut.

Ikke bruk quote_smart på input før du skal bruke de i en spørring. Den er ikke ment for variabler du skal skrive ut, slik som du har gjort det nå. De vil da få quotes rundt seg.
Enten opprett en egen variabel for det som skal inn i databasen, eller bruk det mer dynamisk. Eks. mysql_query("UPDATE blah SET blah=%s", quote_smart($input_string));

Vet ikke helt hvordan din quote_smart ser ut, men dersom du stripper eventuell magic_quotes_gpc der, må du passe på å håndtere den riktig andre steder (dersom den er på). Heldigvis forsvinner den ekle "featuren" snart

Du bør dele opp koden din i en template del og en logikk del. Det er alltid litt glidende overganger, men du har full miks av logikk og presentasjon. Dette gjør koden din veldig uoversiktlig over tid, og tidkrevende å vedlikeholde/utvikle.

Benytt deg også av looper, slik som det er nå kunne du ihvertfall halvert koden, samt gjort den mer oversiktlig, ved å samle like uttrykk (tenker spesielt på validering og utskrift).

Av absolutt småplukk kan det nevnes at $_SESSION (m.fl.) er en variabel. Bruk den som det og ikke putt " rundt den når du sammenligner.
if ($control_panel == $_SESSION['user_id'] && ..
md5($inp_password); // etc.

Ble litt mye det her, og ser kanskje litt krast ut, men det er ikke min intensjon. Jeg tror du kunne hatt stort utbytte av å ta en titt på et rammeverk. Det er en læringskurve og nok litt uvant i begynnelsen, men prøv deg frem med f.eks. CodeIgniter. Det tror jeg ville være midt i blinken.

sindredit · 8 Jul 2008

Aight

Mye å sette seg inn i her;-) Hehe
Skal gå igjennom punkt for punkt å se hvordan jeg skal gjøre det!
Takk for tipset!

Og offtopic:
Nei, jeg selger dette ikke. Jeg gir det bort gratis: http://sindre.ikrsand.net/zipped/sindre-cms.zip

Jeg selger dog andre tjenester: Sindre webutvikling

Mysql sikkerhet..

sindredit

New Member

Vedlegg

Gomlers

Member

tyr897

Medlem

sindredit

New Member