Ankündigung

Einklappen
Keine Ankündigung bisher.

Sicherheitsfrage zu Code

Einklappen

Neue Werbung 2019

Einklappen
X
  • Filter
  • Zeit
  • Anzeigen
Alles löschen
neue Beiträge

  • Sicherheitsfrage zu Code

    Hallo liebes Forum,

    Dank einiger hilfreicher Themen im Forum konnte ich mit meinen PHP Anfängen mein Script wie gewünscht umsetzen.

    Für mich stellt sich als "Neuling" nur die Frage ob der Code auch sicher ist, oder ob ich noch etwas beachten muss.

    Ich habe aus einem anderen Beitrag herausgelesen, dass die Datensätze mit mysql_real_escape_string geprüft werden sollen - habe ich dies richtig angewandt?

    Es handelt sich um ein gewöhnliches Formular, die Daten werden in eine DB geschrieben und nach Erfolg eine Email verschickt.

    Ist es sicherer die DB Zugangsdaten über eine extra Datei einzubinden, oder sie wie im Code zu sehen einfach so mit einzubauen.

    Welche Rechte sollten die Dateien haben= Ich habe sie jetzt auf 644

    Den Code hab ich der Übersicht halber etwas abgespeckt.

    Vielen Dank im Voraus!

    PHP-Code:
    <?php ob_start(); ?>


    <?php include ('formular.php'); ?>

    <?php
    $fehler 
    0;
    $fehlertext "";
    if (isset(
    $_POST["speichern"])) {


    if (!isset(
    $_POST['firma']) Or Trim($_POST['firma']) == "") {
        
    $fehler 1;
        
    $fehlertext "<div id='errmess'>Bitte geben Sie Ihren Firmennamen an.</div>";
        }

    [...]

    if (
    $fehler == 0) {
        
        
        
    $sql " INSERT INTO meldungen ";
        
    $sql .= " SET ";
        
    $sql .= " Firma ='"mysql_real_escape_string($_POST['firma']) ."', ";    

    [...]


    //Zugriff auf die MySQL Datenbank:
    define 'MYSQL_HOST''localhost' );
    define 'MYSQL_BENUTZER''***' );
    define 'MYSQL_KENNWORT''***' );
    define 'MYSQL_DATENBANK''***' );

    $db_link = @mysql_connect (MYSQL_HOSTMYSQL_BENUTZERMYSQL_KENNWORT);

        if ( ! 
    $db_link ){
        die(
    'keine Verbindung zur Zeit möglich ');
        }

    $db_sel mysql_select_dbMYSQL_DATENBANK ) or die("Auswahl der Datenbank fehlgeschlagen");

        

        
    $db_erg mysql_query$sql );
        if ( ! 
    $db_erg ){
        die(
    'Ung&uuml;ltige Abfrage: ' mysql_error());
        }

        if(
    $sql && mysql_affected_rows() > 0){
            
            
            
        
    $firma $_POST['firma'];

    [...]
        
        
    $emailc "Meine Nachricht";
        
        
    $empfanger "info@test.de";
        
    $val mail("$empfanger""Anmeldung""$emailc""From: $email");
        
        
        
    $kundennachricht ="Nachricht für Absender";

        
        
        
    $val2 mail("$email""Ihre Anmeldung""$kundennachricht""From: $empfanger");
        
    header ("Location: ./danke.php");
    exit;
    ob_end_flush();

    }

    }
    else
    echo 
    $fehlertext;
    }
    ?>
    Also es funktioniert alles einwandfrei - möchte nur wissen ob der Code auch Sicherheitstechnisch in Ordnung ist oder ob man noch etwas verbessern könnte

  • #2
    Nein ist er nicht, weil er vermutlich ab PHP 5.6 nicht mehr funktionieren wird und auch bei 5.5 schon Fehler werfen wird.

    Du benutzt veraltete Funktionen (mysql_*), versteckst Fehlermeldungen (@), schreibst funktionen falsch (Trim vs trim, Or vs ||), benutzt generell veraltete Methoden und gefährliche Funktionen, die man besser nicht manuell benutzen sollte (mail) und verlässt dich aus mir unerfindlichen Gründen auf ob_*, was überhaupt nicht nötig wäre. Obendrein verwendest du völlig unnötige String-Variablen-Konstrukte ("$email" anstatt einfach nur $email), bist vermutlich zwar gegen sql-injection gefeit aber vermutlich nicht gegen mail-header-injection und für eine weitere Analyse hast du zuviel von deinem Code weggestrichen.
    [URL="http://goo.gl/6Biyf"]Lerne Grundlagen[/URL] | [URL="http://sscce.org/"]Schreibe gute Beispiele[/URL] | [URL="http://goo.gl/f2jR7"]PDO > mysqli > mysql[/URL] | [URL="http://goo.gl/jvfSZ"]Versuch nicht, das Rad neu zu erfinden[/URL] | [URL="http://goo.gl/T2PU5"]Warum $foo[bar] böse ist[/URL] | [URL="http://goo.gl/rrfzO"]SQL Injections[/URL] | [URL="http://goo.gl/Q81WJ"]Hashes sind keine Verschlüsselungen![/URL] | [URL="http://goo.gl/2x0e2"]Dein E-Mail Regex ist falsch[/URL]

    Kommentar


    • #3
      Hi,

      Also es funktioniert alles einwandfrei - möchte nur wissen ob der Code auch Sicherheitstechnisch in Ordnung ist oder ob man noch etwas verbessern könnte
      Ja, gibt n paar Sachen:

      PHP-Code:
      if (!isset($_POST['firma']) Or Trim($_POST['firma']) == "") { 
      wenn Du ein Formular absendest mit dem Feld und du "nur" prüfen möchtest, ob was eingetragen wurde, reicht auch:

      PHP-Code:
      if(!empty($_POST["firma"])){ //...} 
      Hier

      PHP-Code:
      <?php ob_start(); ?>


      <?php include ('formular.php'); ?>

      <?php
      du brauchst nicht immer öffnen und schließen. Einmal <?php schreiben und dann deinen PHP Code rein.

      PHP-Code:
      ob_start(); 
      Warum brauchst du das hier??

      PHP-Code:
      $db_link = @mysql_connect (MYSQL_HOSTMYSQL_BENUTZERMYSQL_KENNWORT); 
      @-Zeichen vermeiden und lieber eine vernünftige Fehlerverwaltung. Zudem ist mysql_* veraltet. Du solltest auf PDO oder mysqli umsteigen. Mittels Prepared Statements hast Du eine große Arbeitserleichterung und zusätzlich nicht mehr das SQL Injection Problem.

      PHP-Code:
      $val mail("$empfanger""Anmeldung""$emailc""From: $email"); 
      Bitte eine Mailerklasse nutzen und nicht mail(). Ich persönlich nutze Swiftmailer, kannst aber aber auch PHPMailer o.ä. nutzen.

      Das war der erste Eindruck und ich hab bestimmt noch irgendwas übersehen...

      mfg Wolf29
      while (!asleep()) sheep++;

      Unterschätze nie jemanden der einen Schritt zurück geht! Er könnte Anlauf nehmen.

      Kommentar


      • #4
        Vielen Dank für Eure Antworten - gerade @wolf für deine Mühe mir das einzeln aufzulisten.

        Ich werde eure Anmerkungen umsetzen und würde mich freuen, wenn ihr dann nochmal drüber schauen könntet.

        Kommentar

        Lädt...
        X