Panel administracyjny problem z php w formularzu

[denis94 0]

Posiadam panel administracyjny do serwerów shoutcast autopilotów itd.

 

Jest tam formularz dzięki któremu użytkownicy serwerów robią konta swoim prezenterom.

Problem tkwi w tym, że po wpisaniu do tego formularza (np zamiast loginu) kodu php kod ten się wykonuje.

W jaki sposób można to zabezpieczyć ?

 

Proszę o pomoc

[Kolopik 222]

Ja nic z tego nie zrozumiałem. Myślę, że większość użytkowników też.

Wyjaśnij to jeszcze raz. Co chcesz zabezpieczyć? to żeby Kowalski nie mógł założyć konta prezenterowi?

 

 

@Down:

On się chyba boi pokazać kod, bo chce go zabezpieczyć ;P

[kafi 2425]

./szklana_kula "podaj kod_skryptu"
> Error 404: kod_skryptu not found

[Krystian 17]

Zrób filtrowanie wprowadzanych danych w formularzach.

[guziec 109]

Posiadam panel administracyjny do serwerów shoutcast autopilotów itd.

 

Jest tam formularz dzięki któremu użytkownicy serwerów robią konta swoim prezenterom.

Problem tkwi w tym, że po wpisaniu do tego formularza (np zamiast loginu) kodu php kod ten się wykonuje.

W jaki sposób można to zabezpieczyć ?

 

Proszę o pomoc

 

Nawet nie próbuję się domyślać jak wygląda źródło takiego panelu, który ma taką dziurę, że wykona kod podany zamiast loginu. Nie używałbym tego nawet po załataniu, a autora odesłał do jakiejś fizycznej roboty.

 

Ale jak już musisz, to zacznij od tego:

http://www.php.net/manual/en/function.strip-tags.php

http://www.php.net/manual/en/function.htmlentities.php

http://www.php.net/manual/en/function.html-entity-decode.php

[denis94 0]

Tak panel ma sporo dziur. Trochę udało mi się już załatać lecz z tą mam największy problem - formularz wykonuje podany kod lecz nie radzę sobie z tym filtrowaniem. Z linków kolegi guziec, dużo nie zrozumiałem.

 

Mam podać kod całego pliku ? Formularz jak formularz.

<FORM ACTION="" METHOD="post">
<TABLE CELLPADDING="2" CELLSPACING="0" WIDTH="400">
<TR>
<TD WIDTH="200">Login:</TD>
<TD WIDTH="200"><INPUT TYPE="text" NAME="login" ></TD>
</TR>
<TR>
<TD WIDTH="200">Hasło:</TD>
<TD WIDTH="200"><INPUT TYPE="password" NAME="has1" ></TD>
</TR>
<TR>
<TD WIDTH="200">Powtórz hasło:</TD>
<TD WIDTH="200"><INPUT TYPE="password" NAME="has2" ></TD>
</TR>
</TABLE>
<BR>
<INPUT TYPE="submit" VALUE="Zapisz zmiany" NAME="submit">
</FORM>

[MiSi3kK 16]

Ciężko mi wyobrazić sobie co robi skrypt z wartościami z formularza.

Skoro jest to rejestracja to hasła tylko musisz porównać i potem tylko hash.

 

Login to wartość tekstowa i za dużo z nią też robić nie musisz. Addslashes albo http://pl2.php.net/manual/en/function.mysql-real-escape-string.php i nie powinno być żadnych problemów.

[kafi 2425]

Szklana kula chciała by kod skryptu, a nie sam kod formularza...

[denis94 0]

cały plik:

 

<?php
if ($konto == dj_add)
{
if (isset($_POST["submit"]))
{
 if (isset($_POST["login"])){
 if($_POST['has1'] == $_POST['has2']){
$user_id=$_SESSION['id'];
$login = $_POST['login'];
$haslo = $_POST['has1'];
echo'Ustawienia zostały zmienione.';
$query=mysql_query("SELECT login FROM dj WHERE login='".$login."'");
if(mysql_num_rows($query)==0){
$query3 = "INSERT INTO dj VALUES (NULL, '$login', '$haslo', '$user_id', NOW(), NOW())";
$result3 = mysql_query($query3);
echo "Dj został dodany<br>";
}else echo'Taki dj już istnieje<br>';
}else echo'Hasła różnią się od siebie';
}else echo'Podaj login dj';
}
?>
<FORM ACTION="" METHOD="post">
<TABLE CELLPADDING="2" CELLSPACING="0" WIDTH="400">
<TR>
<TD WIDTH="200">Login:</TD>
<TD WIDTH="200"><INPUT TYPE="text" NAME="login" ></TD>
</TR>
<TR>
<TD WIDTH="200">Hasło:</TD>
<TD WIDTH="200"><INPUT TYPE="password" NAME="has1" ></TD>
</TR>
<TR>
<TD WIDTH="200">Powtórz hasło:</TD>
<TD WIDTH="200"><INPUT TYPE="password" NAME="has2" ></TD>
</TR>
</TABLE>
<BR>
<INPUT TYPE="submit" VALUE="Zapisz zmiany" NAME="submit">
</FORM>
<?
}
if($konto==dj){
$query = "SELECT * FROM dj WHERE user_id='{$_SESSION['id']}' ORDER BY id>0";
$result = mysql_query($query);
echo'<table cellpadding="0" cellspacing="0" class="brd" width="700"><tr>
<td>Login</td><td>Data dodania</td><td>Ostatnie logowanie</td><td>Operacje</td></tr>';
while($r = mysql_fetch_array($result)) {
$dj['log'] = $r['log'];
$dj['name'] = $r['login'];
$dj['id'] = $r['id'];
$dj['date'] = $r['date'];
echo'<tr><td>'.$dj['name'].'</td><td>'.$dj['date'].'</td><td>'.$dj['log'].'</td><td>
<a href="?konto=dj_del&user_id='.$dj['id'].'">Usuń</a> | <a href="?konto=dj_edit&user_id='.$dj['id'].'">Edytuj</a> </tr>';
}
echo'</table><br><center><a href="?konto=dj_add"><Dodaj DJ></a></center>';
}
if($konto==dj_del){
$id_dj = $_GET['user_id'];
$query = "DELETE FROM dj WHERE id='$id_dj'";
$result = mysql_query($query);
echo'<center><font color="red">Dj został usunięty</font></center>';
}

if ($konto == dj_edit)
{
if (isset($_POST["edit"]))
{
if($_POST['has1'] == $_POST['has2']){
$login = $_POST['login'];
$haslo = $_POST['has1'];
echo'Ustawienia zostały zmienione.';
$query3 = "UPDATE dj SET haslo='$haslo' WHERE id={$_GET['user_id']}";
$result3 = mysql_query($query3);
echo "Dj został zedytowany<br>";
}else echo'Hasła różnią się od siebie';
}
?>
<FORM ACTION="" METHOD="post">
<TABLE CELLPADDING="2" CELLSPACING="0" WIDTH="400">
<TR>
<TD WIDTH="200">Hasło:</TD>
<TD WIDTH="200"><INPUT TYPE="password" NAME="has1" ></TD>
</TR>
<TR>
<TD WIDTH="200">Powtórz hasło:</TD>
<TD WIDTH="200"><INPUT TYPE="password" NAME="has2" ></TD>
</TR>
</TABLE>
<BR>
<INPUT TYPE="submit" VALUE="Zapisz zmiany" NAME="edit">
</FORM>
<?
}
?>

[guziec 109]

Nie ma w ogóle sprawdzania poprawności danych, kod jest podatny na SQL injection, byle dzieciak zrobi z tego co chce. Wygląda jakby autorem był ktoś kto przeczytał jedynie 'Nauka PHP w Weekend' a i to niezbyt uważnie.

 

Na początek zastosuj to żeby pozwolić tylko na dozwolone znaki

http://php.net/manual/en/function.preg-match.php

I oczywiście nie pozwalaj na większe stringi niż dozwolone:

if (strlen($_POST["login"]) > 12)

if (strlen($_POST["has1"]) > 20) ... itd

powinny skutować informacją o błędach w formularzu.

[MiSi3kK 16]

W kodzie jest tyle błędów, że chyba lepiej napisać to od zera. Zapisywanie hasła jawnie w bazie też nie jest dobrym pomysłem.

 

if ($konto == dj_edit)

 

ten dj_edit to też dziwne rozwiązanie (i każdy taki if). If'y moim zdaniem lepiej zmienić na switch, a wartości tekstowe bierz w apostrofy 'wartosc'.

[kafi 2425]

Ten kod podoba mi się najbardziej

if($konto==dj_del)
{ 
$id_dj = $_GET['user_id']; 
$query = "DELETE FROM dj WHERE id='$id_dj'"; 
$result = mysql_query($query);
echo'<center><font color="red">Dj został usunięty</font></center>';
}

Robiąc prostego foreach od ?id=1 do ?id=9999999 można bardzo łatwo wyczyścić (także NIE SWOJE) konta DJów.

 

Tego nie da się poprawić - to trzeba napisać od nowa uwzględniając przede wszystkim bezpieczeństwo.

Ale za free tego raczej nikt nie zrobi. Bo niestety - taka prawda, że nawet i za miskę ryżu to porządnego skryptu do nie-masowego targetu nie dostaniesz. A jak dostaniesz - to właśnie taki, jak tu pokazany