Sicherheitsrisiken im Arcade MOD?

Allgemeiner Support zum phpBB 2 Board und phpBB 2 Modifikationen
Forumsregeln
Auch wenn hier der Support für phpBB 2 weiterhin aufrecht erhalten bleibt, weisen wir darauf hin, dass das phpBB 2 nicht mehr offiziell unterstützt und weiterentwickelt wird!
Antworten
Twins

Sicherheitsrisiken im Arcade MOD?

Beitrag von Twins »

Ich habe bei mir gerade den CTracker Dateiscanner laufen gelassen und er hat einige Probleme gefunden, u.a. viele beim Arcade MOD v2.1.8 (neuste Version).

Laut Dateiscanner werden die common.php bzw. pagestart.php zu spät eingebunden.
Also habe ich mir die Dateien mal genauer angesehen.

Code: Alles auswählen

define('IN_PHPBB', true);
if(isset($_GET['phpbb_root_path']))
{
    die("Hacking attempt");
}
$phpbb_root_path = './';

include($phpbb_root_path . 'extension.inc');

$filename = basename(__FILE__);
$phpEx    = substr(strrchr(__FILE__, '.'), 1);

include_once($phpbb_root_path . 'common.'.$phpEx);
include_once($phpbb_root_path . 'includes/functions_arcade.'.$phpEx);
include_once($phpbb_root_path . 'includes/bbcode.' .$phpEx);
Stellt dieser Code jetzt ein Sicherheitsrisiko dar?
Was macht das Get da?
Und wieso wird $phpEx extra definiert, wenn es doch in der extensions.inc geladen wird?
Zuletzt geändert von Twins am Mo 23.Jul, 2007 20:21, insgesamt 2-mal geändert.
Benutzeravatar
oxpus
Administrator
Beiträge: 28737
Registriert: Mo 27.Jan, 2003 22:13
Wohnort: Bad Wildungen
Kontaktdaten:

Beitrag von oxpus »

Stellt dieser Code jetzt ein Sicherheitsrisiko dar?
Nein.
Der CT kann nur nicht den "normalen" Aufbau in der Datei erkennen und schlägt daher "Alarm".
Die Zeilen 2 - 5 und 11 sind auch komplett Blödsinn, da eben in Zeile 6 die $phpbb_root_path neu definiert wird (egal, was dann in den Super Globals vorher vorhanden war) und auch $phpEx ist richtigerweise in der extention.inc bereits vorhanden.
Also die Zeilen löschen und der CT sollte die Datei als i. O. ansehen.
Karsten Ude
-={ Das Mädchen für alles }=-
Kein Support per Messenger, Email oder PN! Unaufgeforderte Nachrichten werden ignoriert!
No support per Messenger, Email or PM. Each unasked message will be ignored!
Twins

Beitrag von Twins »

Danke für die Tipps.
Habe die Zeilen gelöscht, die activity.php geht immer noch.

Der CTracker erkennt die Datei aber immer noch als unsicher an, wahrscheinlich wegen diesem Code:

Code: Alles auswählen

$filename = basename(__FILE__);
Kann ich diesen auch löschen? Was genau macht diese Zeile denn?
Benutzeravatar
AmigaLink
Beiträge: 5843
Registriert: Mi 03.Mär, 2004 09:05
Wohnort: NRW
Kontaktdaten:

Beitrag von AmigaLink »

Code: Alles auswählen

$filename = basename(__FILE__);
ermittelt den Namen der aufgerufenen Datei. Damit kann man im weiterem Verlauf des Scriptes sicherstellen das bei Eigenaufrufen der richtige Dateiname genutzt wird, auch wenn dieser mal geändert wurde. Lass es einfach drin und gut is.

Im übrigen finde ich Zeile 2-5 gar nicht sooooo Blödsinnig. Es ist zwar richtig das $phpbb_root_path später explizit definiert und somit das einschleusen eines fremden paths verhindert wird. Aaaber durch Zeile 2-5 wird schon der Versuch einer Manipulation mit dem Abbruch des Scriptes bestraft ...
[center].: Web Relax .::. Essen mit Freude .::. AmigaLink.de :.
______________________________________

Kein Support per PM, ICQ oder eMail!!!
[/center]
Benutzeravatar
oxpus
Administrator
Beiträge: 28737
Registriert: Mo 27.Jan, 2003 22:13
Wohnort: Bad Wildungen
Kontaktdaten:

Beitrag von oxpus »

Aaaber durch Zeile 2-5 wird schon der Versuch einer Manipulation mit dem Abbruch des Scriptes bestraft ...
Sicher, aber warum beenden, wenn die Variable eh neu "korrekt" und sicher erstellt wird?
Karsten Ude
-={ Das Mädchen für alles }=-
Kein Support per Messenger, Email oder PN! Unaufgeforderte Nachrichten werden ignoriert!
No support per Messenger, Email or PM. Each unasked message will be ignored!
Benutzeravatar
AmigaLink
Beiträge: 5843
Registriert: Mi 03.Mär, 2004 09:05
Wohnort: NRW
Kontaktdaten:

Beitrag von AmigaLink »

Warum soll ich es jemandem gestatten mein Script zu nutzen, wenn derjenige versucht es zu Manipulieren? ;)

Kann man sich jetzt drüber Tod-Diskutieren.
Wäre der MOD von mir, dann wären die Zeilen nicht drin. Ich habe sie bei mir aber dennoch drin gelassen. Denn sie schaden nicht, sondern bringen sogar ein wenig nutzen (kein unnötiger Traffic).

Kann jeder für sich selber entscheiden. :)
[center].: Web Relax .::. Essen mit Freude .::. AmigaLink.de :.
______________________________________

Kein Support per PM, ICQ oder eMail!!!
[/center]
Twins

Beitrag von Twins »

Vielen dank.
Ich bin mitterweile bei den Admindateien des Arcade angelangt, wieder wurde die pagestart.php zu spät eingebunden.

Code: Alles auswählen

define('IN_PHPBB', 1);
define('ARCADE_ADMIN', 1);
$file = basename(__FILE__);
if( !empty($setmodules) )
{
// Load DB
  $build_array = array();
  $sql = "SELECT * FROM " . $table_prefix . "ina_data";
 	$result = $db->sql_query($sql);
 	$ina_info = $db->sql_fetchrowset($result);
  foreach ($ina_info as $key => $value)
  {
    $build_array[$value['config_name']]  = $value['config_value'];
  }      
	unset($ina_info);
 	$arcade_config = $build_array;
//  Main Arcade Options
	$module['Arcade']['Configuration'] = $file;
	$module['Arcade']['Switches'] = $file."?mode=switches";
	if($arcade_config['games_moderators_mode'])
	{
		$module['Arcade']['Moderators'] = $file."?mode=moderators";
	}
	if($arcade_config['games_use_pms'])
	{
		$module['Arcade']['Private MS'] = $file."?mode=messages";
	}
	if($arcade_config['games_tournament_mode'])
	{
		$module['Arcade']['Tournaments'] = "admin_arcade_tournaments.$phpEx";
	}
	return;
}
$phpbb_root_path = './../';
require($phpbb_root_path . 'extension.inc');
require('pagestart.' . $phpEx);
include_once($phpbb_root_path . 'includes/functions_arcade.'.$phpEx);
Also die Konstanten Zeile 1-2 sind nicht unsicher, haben andere Admindateien auch. Irgendwo hatte mal einer geschrieben, dass Konstante nicht unsicher sein können.
Zeile 3-4 ist normal, haben andere Dateien auch.

Nur Load DB und Main Arcade Options... Kann man diese Funktionen nicht unter die pagestart.php verschieben?
Und ist es egal, ob man bei $phpbb_root_path immer " benutzt oder '?
Benutzeravatar
oxpus
Administrator
Beiträge: 28737
Registriert: Mo 27.Jan, 2003 22:13
Wohnort: Bad Wildungen
Kontaktdaten:

Beitrag von oxpus »

Lass alles so, wie es ist.
Da ist kein Sicherheitsrisiko drinnen.
Der CT meckert hier sicher nur an, daß in der Menüfunktion des Modules mehr drinnen steht, als normal üblich.
Aber hier ist das okay so.

Und der Unterschied zwischen " und ' ist nicht einfach zu verstehen.
Während ein String mit " eingeschlossen alle enthaltenen Variablen umwandelt und deren Inhalte verwendet, den String also vor der Verwendung "parst", werden Strings in ' nur genau so wie angegeben dargestellt.
Letzterer ist damit auch schneller (wenngleich nicht wirklich spürbar), kann aber auch zu ungewollten Ergebnissen führen.
Man sollte sich aber angewöhnen, immer möglichst nur einfache ' zu verwenden. " sind aber nicht falsch, wenn man sie korrekt anwendet...
Karsten Ude
-={ Das Mädchen für alles }=-
Kein Support per Messenger, Email oder PN! Unaufgeforderte Nachrichten werden ignoriert!
No support per Messenger, Email or PM. Each unasked message will be ignored!
Twins

Beitrag von Twins »

Danke.
Der CTracker erkennt also nur, dass der Code vom Standard abweicht?
Immerhin wird da auch der Debug-Code als unsicher erkannt, was er aber nicht ist.

Eine Frage habe ich aber nur noch zur admin_megamail.php:

Code: Alles auswählen

$def_wait = 10;
$def_size = 100;

define('IN_PHPBB', 1);

if( !empty($setmodules) )
{
	$filename = basename(__FILE__);
	$module['General']['Mega Mail'] = $filename;
	
	return;
}

// Load default header
$no_page_header = TRUE;
$phpbb_root_path = './../';
require($phpbb_root_path . 'extension.inc');
require('./pagestart.' . $phpEx);
include($phpbb_root_path . 'language/lang_' . $board_config['default_lang'] . '/lang_megamail.' . $phpEx);

define('MEGAMAIL_TABLE', $table_prefix.'megamail');
Was haltet ihr hier von? Habe keine Ahnung, was $def_wait macht, aber unsicher ist die Datei deswegen nicht, oder?
Benutzeravatar
oxpus
Administrator
Beiträge: 28737
Registriert: Mo 27.Jan, 2003 22:13
Wohnort: Bad Wildungen
Kontaktdaten:

Beitrag von oxpus »

Solange das nicht in einer SQL-Abfrage vorkommt, ist es wurscht.
Wobei es immer besser ist, erst nach dem Einbinden der allgemeinen Dateien (hier nach dem require für die pagestart.php) Variablen festzulegen...
Karsten Ude
-={ Das Mädchen für alles }=-
Kein Support per Messenger, Email oder PN! Unaufgeforderte Nachrichten werden ignoriert!
No support per Messenger, Email or PM. Each unasked message will be ignored!
Twins

Beitrag von Twins »

Und was soll ich bei Dateien machen, wo es überhaupt kein include der common.php gibt?

attach_mod/includes/functions_filetypes.php
includes/rewards_api.php

Dort in der kompletten Datei wird keine common.php eingebunden.
Benutzeravatar
cbrkiter
Beiträge: 170
Registriert: Fr 26.Nov, 2004 01:30
Kontaktdaten:

Beitrag von cbrkiter »

Sorry, wenn ich es in dieses Thema schreibe, aber mir ist gerade eine generelle Frage eingefallen, nachdem ich folgendes gelesen habe:

[quote="AmigaLink";p="75258"]Im übrigen finde ich Zeile 2-5 gar nicht sooooo Blödsinnig. Es ist zwar richtig das $phpbb_root_path später explizit definiert und somit das einschleusen eines fremden paths verhindert wird. Aaaber durch Zeile 2-5 wird schon der Versuch einer Manipulation mit dem Abbruch des Scriptes bestraft ...[/quote]

Wenn ich doch die Übergabe der Variable komplett verhinden (bzw. abfangen) möchte und die Aufrufe grundsätzlich ins Nirvana schicken möchte... oder sagen wir mal so, mein Server soll sich damit nicht weiter belasten, dann kann ich dies doch bestimmt schon mit der htaccess erledigen. Ich stelle mir dies also so vor, dass egal welcher Aufruf mit einer Angabe der Variablen direkt umgeleitet wird auf 127.0.0.1 oder so ähnlich.

a) Macht dies Sinn, um evtl. Sicherheitslöcher von MODs auszubügeln?
b) Wie müsste die Anweisung in der htacess aussehen?
1. 'Man muss das Unm?gliche versuchen, um das M?gliche zu erreichen!' (Hermann Hesse)
2. 'The best way to learn something is to get your hands dirty and do it!' (keine Ahnung))
3. http://www.thw-kamen-bergkamen.de
Benutzeravatar
oxpus
Administrator
Beiträge: 28737
Registriert: Mo 27.Jan, 2003 22:13
Wohnort: Bad Wildungen
Kontaktdaten:

Beitrag von oxpus »

@Twins
Die common.php muss nicht immer in der Datei direkt eingebunden werden, das kann auch in einer andere Datei passieren ;)

@cbrkiter
a) Jein, kommt darauf an, ob der MOD weiter entwickelt wird oder nicht und wenn nein, ob er wirklich soooo wichtig ist, daß man dessen "Fehler" korrigiert.
b) In der .htaccess sollte man die Post-Daten besser nicht behandeln, da der Webserver hier selber dann mit der Analyse beschäftigt ist und nicht ein anderes Modul wie hier PHP. Der Webserver hat schliesslich andere Aufgaben, daß Post- oder Get-Daten zu prüfen ;)
Karsten Ude
-={ Das Mädchen für alles }=-
Kein Support per Messenger, Email oder PN! Unaufgeforderte Nachrichten werden ignoriert!
No support per Messenger, Email or PM. Each unasked message will be ignored!
Benutzeravatar
cbrkiter
Beiträge: 170
Registriert: Fr 26.Nov, 2004 01:30
Kontaktdaten:

Beitrag von cbrkiter »

Ok - dann will ich dies mal nicht zu sehr vertiefen. Aber eine Frage hätte ich noch, macht es so viel Unterschied, ob der Webserver sich der Aufgabe direkt annimmt oder ein Modul, dass auf dem Server läuft? Ich weiß, ich habe hier Null Ahnung, aber vielleicht könnt Ihr diese im Nachkommabereich ein wenig erhöhen ^2
1. 'Man muss das Unm?gliche versuchen, um das M?gliche zu erreichen!' (Hermann Hesse)
2. 'The best way to learn something is to get your hands dirty and do it!' (keine Ahnung))
3. http://www.thw-kamen-bergkamen.de
Benutzeravatar
oxpus
Administrator
Beiträge: 28737
Registriert: Mo 27.Jan, 2003 22:13
Wohnort: Bad Wildungen
Kontaktdaten:

Beitrag von oxpus »

Sagen wir es mal allgemeiner:
Je mehr "zentrale" Aufgaben etwas hat, desto weniger sollte es sich mit Details beschäftigen.
Einer CPU sollte man z. B. möglichst wenige Grafikberechungen überlassen, um den Rechner insgesamt nicht auszubremsen, einen Webserver eben die Darstellung und Auslieferung der Webseiten überlassen und nicht die Daten zu prüfen, sofern das andere, eingebundene Module übernehmen können, die damit dem Webserver überlassen.
Daher eben besser keine Prüfung von POST-/GET-Daten in der .htaccess, zumal ich jetzt sogar noch nicht einmal wüsste, ob und wie das überhaupt geht...
Karsten Ude
-={ Das Mädchen für alles }=-
Kein Support per Messenger, Email oder PN! Unaufgeforderte Nachrichten werden ignoriert!
No support per Messenger, Email or PM. Each unasked message will be ignored!
Benutzeravatar
cbrkiter
Beiträge: 170
Registriert: Fr 26.Nov, 2004 01:30
Kontaktdaten:

Beitrag von cbrkiter »

Also ich hätte jetzt an sowas gedacht:

Code: Alles auswählen

RewriteCond %{QUERY_STRING} ^(.*)phpbb_root_path(.*)
RewriteRule ^.*$ http://127.0.0.1/ [R,L]
Das habe ich mir jetzt aber nur zusammenkopiert. Keine Ahnung, ob dies funktionieren würde. Da zwecks Sicherheit eh schon ein paar Dinge in der .htaccess abgefragt werden, hätte ich für die eine Kontrolle mehr oder weniger keine Performancebedenken gehabt.

EDIT:
Drüber nachdenken tue ich eigentlich auch nur deswegen, weil der ct bei mir im log ständig genau diese Anfragen stoppt. Ich meine, dafür ist er ja auch u. a. da, aber wenn der sich erst garnicht damit beschäftigen müsste, dann müsste es meinen Server doch auch entlasten - oder?
1. 'Man muss das Unm?gliche versuchen, um das M?gliche zu erreichen!' (Hermann Hesse)
2. 'The best way to learn something is to get your hands dirty and do it!' (keine Ahnung))
3. http://www.thw-kamen-bergkamen.de
Benutzeravatar
oxpus
Administrator
Beiträge: 28737
Registriert: Mo 27.Jan, 2003 22:13
Wohnort: Bad Wildungen
Kontaktdaten:

Beitrag von oxpus »

Der Server wird nur entlastet, weil er die Analyse nicht fahren muss, daß macht ja PHP mit dem CT. Aber die Anfrage schlägt dennoch beim Webserver aus und er muss die bearbeiten. Das geht dann nur mit Modulen, die mod_evasive oder anderen Sicherheitsmodulen für den Indianer, die man aber dann einzustellen wissen muss, um nicht die Seite komplett lahm zu legen.
Karsten Ude
-={ Das Mädchen für alles }=-
Kein Support per Messenger, Email oder PN! Unaufgeforderte Nachrichten werden ignoriert!
No support per Messenger, Email or PM. Each unasked message will be ignored!
Benutzeravatar
cbrkiter
Beiträge: 170
Registriert: Fr 26.Nov, 2004 01:30
Kontaktdaten:

Beitrag von cbrkiter »

ok - ich bin raus ^a - du hast gewonnen ;)

PS: Sorry - bin halt manchmal ein Querdenker ohne Ahnung ...
1. 'Man muss das Unm?gliche versuchen, um das M?gliche zu erreichen!' (Hermann Hesse)
2. 'The best way to learn something is to get your hands dirty and do it!' (keine Ahnung))
3. http://www.thw-kamen-bergkamen.de
Benutzeravatar
oxpus
Administrator
Beiträge: 28737
Registriert: Mo 27.Jan, 2003 22:13
Wohnort: Bad Wildungen
Kontaktdaten:

Beitrag von oxpus »

Hier gehts nicht ums gewinnen oder verlieren, sondern um Logik.
Man kann sowas auch klar dem Webserver überlassen, wenn dieser immer genügend Performance dafür frei hat.
Aber es wäre eben ungeschickter...
Karsten Ude
-={ Das Mädchen für alles }=-
Kein Support per Messenger, Email oder PN! Unaufgeforderte Nachrichten werden ignoriert!
No support per Messenger, Email or PM. Each unasked message will be ignored!
Twins

Beitrag von Twins »

Okay, danke oxpus.
Eine hoffenletze letze Frage habe ich.

Bei der Datei pnFlashGames.php im Root meldet der CTracker, dass ein undefinierbarer Fehler bein Scannen aufgetreten ist.
Soweit ich sehen kann, besteht die Datei zu fast 90% aus einen Dateikommentar oder Codes, welche auskommentiert wurden.

Und die 10% Code sind eben der "Grundstock" jeder Datei, wie das includen der common.php und extension.inc, etwas später wird die includes/functions_arcade.php eingebuden.

Ich hänge die Datei mal aos Anhang kann.
Wieso tritt dort beim Scanner ein Fehler auf?
Vielleicht, weil außer der Grundbasis nur Kommentar in der Datei vorhanden ist?

Und was haltet ihr hier von (profile.php)?

Code: Alles auswählen

define('IN_PHPBB', true);
if ( (isset($HTTP_GET_VARS['mode']) && ($HTTP_GET_VARS['mode'] == 'viewprofile')) || (isset($HTTP_POST_VARS['mode']) && ($HTTP_POST_VARS['mode'] == 'viewprofile')) )
{
	define('IN_CASHMOD', true);
	define('CM_VIEWPROFILE',true);
}
$phpbb_root_path = './';
include($phpbb_root_path . 'extension.inc');
Habe gerade das in meiner quick_replay-php gefunden:

Code: Alles auswählen

$mode = $_GET['mode'];
$phpbb_root_path = "./";

if ( $mode == 'smilies' )
{
	define('IN_PHPBB', true);
  define('CT_SECLEVEL', 'LOW');
	include($phpbb_root_path . 'extension.inc');
	include($phpbb_root_path . 'common.'.$phpEx);
	include($phpbb_root_path . 'includes/functions_post.'.$phpEx);
	generate_smilies('window', PAGE_POSTING);
	exit;
}

if ( !defined('IN_PHPBB') )
{
	die('Hacking attempt!');
}
define('CT_SECLEVEL', 'MEDIUM');
$ct_ignorepvar = array('submit', 'helpbox');
include($phpbb_root_path . 'common.'.$phpEx);
Du hast keine ausreichende Berechtigung, um die Dateianhänge dieses Beitrags anzusehen.
Zuletzt geändert von Twins am Mi 25.Jul, 2007 15:45, insgesamt 4-mal geändert.
Benutzeravatar
cbrkiter
Beiträge: 170
Registriert: Fr 26.Nov, 2004 01:30
Kontaktdaten:

Beitrag von cbrkiter »

Hier gehts nicht ums gewinnen oder verlieren, sondern um Logik.
Schon klar, es war auch eher als Wortspiel gedacht. Kurz gesagt: Du hast mich überzeugt.

***OffTopic ENDE***
1. 'Man muss das Unm?gliche versuchen, um das M?gliche zu erreichen!' (Hermann Hesse)
2. 'The best way to learn something is to get your hands dirty and do it!' (keine Ahnung))
3. http://www.thw-kamen-bergkamen.de
Benutzeravatar
oxpus
Administrator
Beiträge: 28737
Registriert: Mo 27.Jan, 2003 22:13
Wohnort: Bad Wildungen
Kontaktdaten:

Beitrag von oxpus »

In der profile.php würde ich den Code der Zeilen 2-6 eher nach

Code: Alles auswählen

//
// End session management
//
verschieben.
Ausser, die Konstanten werden bereits durch die eingebundenen Standard-Dateien des phpBB verwendet.
Das kann ich aber nicht beurteilen...

Bei der quick_reply.php ist das aber so korrekt, sonst würde unnötiger Code ausgeführt, wenn man das dort enthaltene Smilies-Window öffnen wollte.

An der pnFlashGames.php kann ich auch nichts Negatives entdecken.
Ausser, daß in der Datei alles in einer Zeile steht.
Die muss dringend reformatiert werden, sonst kann die nicht wirklich korrekt funktionieren...
Am besten mit Wordpad öffnen und speichern, dann sollten die Zeilenumbrüche wieder da sein...
Karsten Ude
-={ Das Mädchen für alles }=-
Kein Support per Messenger, Email oder PN! Unaufgeforderte Nachrichten werden ignoriert!
No support per Messenger, Email or PM. Each unasked message will be ignored!
Twins

Beitrag von Twins »

Vielen dank!

Habe jetzt alle Dateien bei mir angepasst, unsichere wurden abgeändert (jetzt sind die sicher) und andere wurden als sicher eingestuft.
Der CTracker zeigt nun überall sicher an.

Danke an euch beiden für diese tolle Hilfe!! :respect:
Benutzeravatar
oxpus
Administrator
Beiträge: 28737
Registriert: Mo 27.Jan, 2003 22:13
Wohnort: Bad Wildungen
Kontaktdaten:

Beitrag von oxpus »

Kein Problem und schön, daß es endlich auch mal einer geschafft hat.
Das gibt mir Mut bei meinem Forum :D :( ;)
Karsten Ude
-={ Das Mädchen für alles }=-
Kein Support per Messenger, Email oder PN! Unaufgeforderte Nachrichten werden ignoriert!
No support per Messenger, Email or PM. Each unasked message will be ignored!
Antworten