phpbb3: Integration Webseite / Schutz vor URL Injection

[Wuppi]

Hi

hab fürs phpBB3 eine kleine Linkliste gebastelt. Da mein php eher rudimentär ist, ist das ganze vermutlich eher quick&dirty

Aufruf: linkliste.php (zugleich Hauptmenu)
Aufruf der Kategorien: linkliste.php?menupunkt=1
Aufruf weiterer Seiten einer Kategorie: linkliste.php?menupunkt=1&start=1

Für die Integration hab ich die Anleitung hier im Forum genommen. Für die Pagination hab ich ebenfalls die Forenfunktion genutzt.

Datenbankaufruf bzw. Kategorie-Ansteuerung mußte aber selber kreiiert werden - und hier möchte ich ein wenig Sicherheit reinbasteln. start wird sicher sein, da ich hier voll die paginationfunktion vom Forum nutze.

Code: Alles auswählen

// vorbereitung PAGINATION
		$mode = request_var('menupunkt', '');
		$params[] = "menupunkt=$mode";
		$pagination_url = append_sid("{$phpbb_root_path}".$dateiname, implode('&', $params));
// vorbereitung PAGINATION _ ENDE

// erstellt Seitenzahlen
		$template->assign_vars(array(
			'PAGINATION'	=> generate_pagination($pagination_url, $Anzahl, $ErgebnisseProSeite, $start),
			'PAGE_NUMBER'	=> on_page($Anzahl, $ErgebnisseProSeite, $start)
			));
// erstellt Seitenzahlen  - ENDE

Werde ich daher wohl nicht weiter sichern müssen?

Meine erste Idee war mod_rewrite zu nutzen. Aus linkliste.php?menupunkt=1&start=1 wird Links_1-1.html ... per Mod_Rewrite kann ich ja sagen das 1-1 ausschließlich zahlen enthalten kann - sonst matched es nicht und es landet in einem 404. So hab ich das bei meiner Privaten Webseite auch gemacht . Aber da ich beim phpBB3 auf "statische URLs"-SEO verzichten möchte, möchte ich die idee wieder verwerfen.

Da so jeder weiß wie die php-Seite heißt und was für Variablen wir hier haben, kann jetzt entsprechende Angriffschemata starten. Da "menupunkt" ausschließlich aus Zahlen bestehen kann, könnte ich hier ganz am Anfang folgendes abfragen:

Code: Alles auswählen

if (is_numeric($_GET[menupunkt])) 
{} else exit;

Somit würde bei illegalen Aufruf mit was auch immer der Aufbau der Seite abgebrochen, da is_numeric keine Zahl enthält.

Leider greift dieser "Schutz" auch bei linkliste.php - also der Startseite Keine Variable -> somit illegaler Aufruf

Wie kann ich das umgehen bzw. richtig machen? Was sollte ich hier noch beachten?

Gibt es paar Methoden wie man seine Scripte testen kann? Vermutlich nicht? ... bzw. die wird mir natürlich keiner nennen, da man so ja direkt ne "waffe" in die Hand bekommt?

Gruß

[Helmut]

Hallo Wuppi,

warum verwendest du nicht die eingebaute Funktion "generate_pagination" zur Anzeige deiner Linkseiten?

Hier findest du etwas Lesestoff dazu http://wiki.phpbb.com/Generate_pagination

Ich habe damit auch einige Seiten für meine neue Homepage ausgestattet. Das sollte auch bei dir gehen, da du deine Links ebenfalls in einer Datenbank hast.

Gruß Helmut

[gn#36]

Das erste was mir an der Kombination auffällt ist: Wenn der If Zweig leer ist, dann negier ihn einfach und packe alles was du beim else machen willst ins if. Das zweite ist: Wenn du menupunkt nicht als Konstante definiert hast (define('menupunkt', xyz); ) dann solltest du anführungszeichen benutzen, wenn es eine Variable ist fehlt ein $ Zeichen. Du kannst außerdem wenn du willst auch einfach per isset() prüfen ob die Variable existiert. Helmuts Tipp hat aber definitiv auch was für sich

Außerdem: request_var() überprüft Datentypen. Wenn du als defaultparameter eine 0 übergibst wirst du bei Strings grundsätzlich deinen Defaultwert zurückerhalten. Andere Werte werden nicht zugelassen, du kannst dir die Benutzung also sparen wenn du mit request_var arbeitest.

[Wuppi]

Hallo Wuppi,

warum verwendest du nicht die eingebaute Funktion "generate_pagination" zur Anzeige deiner Linkseiten?

Hier findest du etwas Lesestoff dazu http://wiki.phpbb.com/Generate_pagination

Ich habe damit auch einige Seiten für meine neue Homepage ausgestattet. Das sollte auch bei dir gehen, da du deine Links ebenfalls in einer Datenbank hast.

Gruß Helmut

Pagination vom phpBB3 nutz ich ja zur Durchnummerieung der Unterseiten (Seite 1..2..3..4; wie hier beim Forum in der Memberlist oder wenn nen Thread die nächste Seite erreicht) oder kann ich damit auch den kompletten aufbau der Seite basteln? Also das rausziehen der Links (sind glaub 6 Spalten in der Datenbank die ausgelesen und ausgeben werden müssen) ebenfalls darüber. Das script zum numieren kommt von besagter Seite - nur das das mehr als als Seitenzahlen anzeigen wußte ich net

Das Linklistenscript ist nur schon komplett fertig. Kategorieren, Seitenzahlen, Banner, Beschreibung usw. Wenn ich pagination auch zum anzeigen von Content nutzen kann, müsste ich komplett neu machen - dafür hab ich leider derzeit nicht so die Zeit aka Motivation. Würde das aber in der nächsten Version (paar Ideen hab ich da schon die aktuell nicht implementierbar sind) dann ggf. anwenden.

@gn#36:
du verwirrst mich

Code: Alles auswählen

if (is_numeric($_GET[menupunkt]))
{} else exit;

also hier kein {} ("was soll er machen wenn if zutrifft") sondern direkt fragen "wenn is_numeric NICHT zutrifft, dann exit" (deutsche Schreibweise *G*; in php muß ich raussuchen wie es geht - war das die geschichte mit dem "!" --- !is_numeric ?).

Ist keine Konstante (weiß net wie man mit sowas arbeitet). Ist halt ne Variable. Aber wo fehlt dann das $?
$_GET[menupunkt] ... meinst eher $_GET["menupunkt"] ... aber wohin das $? Ich setz ja weiter im Script dann $_GET[menupunkt] zu einer lokalen Variable um ( $menupunkt = $_GET[menupunkt]; )

Das mit isset ist natürlich eher die leichte Lösung ... nutz den befehl schon recht oft anderswo ... warum der mir hier nicht gekommen ist - k.a.
Frag also ob menupunkt gesetzt ist, wenn ja - is_numeric prüfen - isses keine zahl, dann exit; ansonsten normal weiter.

request_var() kenn ich bisher nicht => http://wiki.phpbb.com/Request_var ... hilft mir nicht so weiter. Was bringt mir request_var?

Aber frage geht ja mehr in die Richtung des Angriffs. Welche befehle sind "Angriffsresitent"? ... ich hab von der Materie absolut keine Ahnung ... aber bei so manchen Seiten kann man über die URL code einschleußen: menupunkt=!=§()%/$§)=$/(http://www.bösesscript.tld/script.txt (exemplarisch *G*)... und schwupps wird beim aufruf das script "lokal" ausgeführt. Da ich wie schon gesagt: Quick & Dirty-Programmierer bin , lass ich hier bestimmt auch irgendwo Angriffsraum offen, weil ich nicht weiß wie man das verhindert und einfaches Schulbuch-PHP mache -- bei ner Seite mit fast 10.000 Usern möcht ich das doch etwas sicherer gestalten ...)

Gruß

[gn#36]

Das Ausrufezeichen negiert, !is_numeric() ist also das genaue Gegenteil von is_numeric().

request_var() ist in phpBB die Funktion die sämtliche Variablenzugriffe auf $_GET, $_POST und $_COOKIE kapseln sollte. Die Funktion soll sicherstellen dass du genau das bekommst was du erwartest und nicht irgendwelchen Müll den böse Hacker sich ausdenken. Du kannst ihr mit dem ersten Parameter den Namen der gewünschten Variablen geben (menupunkt oder wie auch immer), mit dem zweiten gibst du ihr den Datentyp, indem der Defaultwert genau den Datentyp hat. Beispiel:

Code: Alles auswählen

$page = request_var('page', 0);
$mode = request_var('mode', '');
$text = request_var('text', '', true);

Das erste Beispiel sucht nach numerischen Parametern in $_GET und $_POST. Findet es die nicht gibt es eine 0.
Das zweite Beispiel hingegen sucht nach einem String ohne Sonderzeichen (Sonderzeichen = äöüß, nicht Punkt, Komma Strich).
Das dritte Beispiel sucht nach einem String der auch Sonderzeichen enthalten darf.

Um beim ersten Beispiel zu bleiben wäre das so was ähnliches wie:

Code: Alles auswählen

$page = isset($_GET['page'])? $_GET['page'] : 0;
if(!is_numeric($page))
{
    $page = 0;
}

Nur eben alles kompakt in einem Aufruf (und nicht genau so umgesetzt wie dieses Beispiel, aber das sollte erst mal nicht weiter stören ).

Jeder an diese Variable weitergegebene nicht numerische Inhalt resultiert in einer 0 als Wert für die Variable (= ein erlaubter Wert). Bei Strings musst du grundsätzlich immer aufpassen was du damit machst, es ist quasi unmöglich alle Eventualitäten mit einer Überprüfung des Inhalts abzufangen, du musst den Inhalt eben so vorsichtig verwenden, dass nichts passieren kann (kein direkter Einbau in irgendwelche SQL Abfragen u.ä.)
Ein URL Include sollte bei phpBB eigentlich so nicht möglich sein, es sei denn ein veralteter Mod (oder eine unfertige Entwicklungsversion) hat eine gravierende Sicherheitslücke.

[Helmut]

Hallo Wuppi,

klar kannst du mit der Funktion noch mehr machen als nur die Seitenzahlen generieren. Ich verwende das für mein Gästebuch um aus einem Forum die Beiträge eines bestimmten Topics anzeigen zu lassen. Hier mal ein kleines Beispiel:

Code: Alles auswählen

[...]
$forum_id = 11;
$topic_id = 11;

$start        = request_var('start', 0);
$post_id        = request_var('p', 0);

//Navigation
$sql = 'SELECT COUNT(post_id) AS total_posts
    FROM ' . POSTS_TABLE . "
    WHERE topic_id = $topic_id";
    $result = $db->sql_query($sql);
    $total_posts = (int) $db->sql_fetchfield('total_posts');
    $db->sql_freeresult($result);
    
    // If we've got a hightlight set pass it on to pagination.
    $pagination = generate_pagination(append_sid("{$phpbb_root_path}book.$phpEx", "f=$forum_id&t=$topic_id" . (($highlight_match) ? "&hilit=$highlight" : '')), $total_posts, $config['posts_per_page'], $start);


    $template->assign_vars(array(
            'PAGINATION'         => $pagination,
            'PAGE_NUMBER'         => on_page($total_posts, $config['posts_per_page'], $start),            
            'TOTAL_POSTS'        => ($total_posts == 1) ? $user->lang['VIEW_TOPIC_POST'] : sprintf($user->lang['VIEW_TOPIC_POSTS'], $total_posts),            
                
            'S_TOPIC_ACTION'     => append_sid("{$phpbb_root_path}book.$phpEx", "f=$forum_id&t=$topic_id&start=$start"), 
                        
    ));

    $store_reverse = false;
    $sql_limit = $config['posts_per_page'];
    if ($start > $total_posts)
    {
        $store_reverse = true;

        if ($start + $config['posts_per_page'] > $total_posts)
        {
            $sql_limit = min($config['posts_per_page'], max(1, $total_posts - $start));
        }

        // Select the sort order
        $sql_start = max(0, $total_posts - $sql_limit - $start);
    }
    else
    {
        $sql_start = $start;
    }

//Textausgabe
$post_list = array();
$i = $i_total = 0;

$sql = 'SELECT p.post_subject, p.post_text, p.post_id, poster_id, u.user_id, u.username, p.post_time, p.post_username, bbcode_uid, bbcode_bitfield, enable_bbcode, enable_smilies, enable_magic_url
    FROM ' . POSTS_TABLE . " p
    LEFT JOIN " . USERS_TABLE . " u
        ON u.user_id = p.poster_id
    WHERE p.topic_id = $topic_id
    ORDER BY p.post_id DESC";
$result = $db->sql_query_limit($sql, $sql_limit, $sql_start);
$i = ($store_reverse) ? $sql_limit - 1 : 0;
while ($row = $db->sql_fetchrow($result))
{
    $post_list[$i] = $row['post_id'];
    ($store_reverse) ? $i-- : $i++;
    
    //set some default vars
    $post_id = $row['post_id'];
    $poster_id = $row['poster_id'];
    $username = $row['username'];
        
    $row['bbcode_options'] = (($row['enable_bbcode']) ? OPTION_FLAG_BBCODE : 0) + (($row['enable_smilies']) ? OPTION_FLAG_SMILIES : 0) + (($row['enable_magic_url']) ? OPTION_FLAG_LINKS : 0);    
    $row['post_text'] = generate_text_for_display($row['post_text'], $row['bbcode_uid'], $row['bbcode_bitfield'], $row['bbcode_options']);        


    $template->assign_block_vars('book', array(
            'POST_TITLE'        => censor_text($row['post_subject']),
            'POST_TEXT'        => censor_text($row['post_text']),
            'POST_DATE'        => $user->format_date($row['post_time']),
            'POST_ID'            => $row['post_id'],
            'POST_AUTHOR_FULL'    => get_username_string('full', $row['poster_id'], $autor),

            'U_POST_AUTHOR'    => get_username_string('profile', $row['poster_id'], $row['username']),
            'U_GB_POST'        => append_sid("{$phpbb_root_path}book.$phpEx" , 'p=' . $row['post_id'] . '#p' . $row['post_id']),
    ));        
}

[...]
 

Über $config['posts_per_page'] steuerst du, wieviele Posts oder bei dir Links auf einer Seite angezeigt werden sollen. Du kannst die Abfrage deiner Datenbank auch beliebig erweitern und noch weitere Dinge anzeigen lassen. Die Ausgabe geschieht wie üblich über die *.html Datei.

Code: Alles auswählen

        <!-- IF PAGINATION or TOTAL_POSTS -->
            <div class="pagination">
                {TOTAL_POSTS}
                <!-- IF PAGE_NUMBER --><!-- IF PAGINATION --> &bull; <a href="#" onclick="jumpto(); return false;" title="{L_JUMP_TO_PAGE}">{PAGE_NUMBER}</a> &bull; <span>{PAGINATION}</span><!-- ELSE --> &bull; {PAGE_NUMBER}<!-- ENDIF --><!-- ENDIF -->
            </div>
        <!-- ENDIF -->
 

Ich denke, damit kommst du soweit klar, eine Liste von Links anzeigen zu lassen ist ja vergleichbar mit der Anzeige von Posts pro Topic.

Gruß Helmut

[Wuppi]

Hi

@gn#36: jetzt isses klarer Danke

@Helmut: ich denke mal ich werde es verstehen wenn ich damit mal bissel rumspiele. Hab da ggf. auch schon ein kleines Projektchen. Muß noch ne Zusatzseite vom aktuellen Forum nach phpBB3 migrieren Die Community weigert sich mit Händen und Füßen das Thema in ein Wiki, welches wir haben und wo es definitiv hingehört, zu überführen (man könnte auch sagen: sie ignoriert es) ... also darf ich es doch wieder ins Forum packen - wenn ich es net mache, bekomm ich nach Update massig Mails "wo ist das!?" ... tja die leiden eines Forenadmins

Gruß

[Wuppi]

@gn#36

Code: Alles auswählen

$menupunkt = request_var('menupunkt', 0);
if ($menupunkt == '0') { unset($menupunkt); }

so hab ich es jetzt relativ weit oben angebrach. Unset deswegen weils keine 0er Kategorie gibt und meine Abfragen keine Null vorsehen - so der einfachste weg.

Zum test nachm unset noch nen echo $menupunkt; gepappt - und Ergebniss ist wie erwartet. Lande im Hauptmenu, Testausgabe (?menupunkt=alslslslsl) gibt nix zurück. Also passend Danke

Gruß