SQL ERROR in Topposters Extension

[Kirk]

Hallo
Wenn ich in meiner Topposters Extension im ACP Modul bei "User IDs ausschließen" eine User ID (z. b. die 49) eintrage, bekomme ich im index folgenden SQL Error:

Code: Alles auswählen

SQL ERROR [ mysqli ]

You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '\'49\' ORDER BY user_posts DESC LIMIT 3' at line 6 [1064]

SQL

SELECT username, user_id, user_type, user_colour, user_posts FROM phpbb_users WHERE user_id <> 1 AND user_type <> 2 AND user_posts > 0 AND user_id <> \'49\' ORDER BY user_posts DESC LIMIT 3

BACKTRACE

FILE: (not given by php)
LINE: (not given by php)
CALL: msg_handler()

FILE: [ROOT]/phpbb/db/driver/driver.php
LINE: 1023
CALL: trigger_error()

FILE: [ROOT]/phpbb/db/driver/mysqli.php
LINE: 195
CALL: phpbb\db\driver\driver->sql_error()

FILE: [ROOT]/phpbb/db/driver/mysql_base.php
LINE: 45
CALL: phpbb\db\driver\mysqli->sql_query()

FILE: [ROOT]/phpbb/db/driver/driver.php
LINE: 296
CALL: phpbb\db\driver\mysql_base->_sql_query_limit()

FILE: [ROOT]/phpbb/db/driver/factory.php
LINE: 337
CALL: phpbb\db\driver\driver->sql_query_limit()

FILE: [ROOT]/ext/kirk/topposters/event/listener.php
LINE: 187
CALL: phpbb\db\driver\factory->sql_query_limit()

FILE: [ROOT]/vendor/symfony/event-dispatcher/EventDispatcher.php
LINE: 214
CALL: kirk\topposters\event\listener->display_topposters()

FILE: [ROOT]/vendor/symfony/event-dispatcher/EventDispatcher.php
LINE: 44
CALL: Symfony\Component\EventDispatcher\EventDispatcher->doDispatch()

FILE: [ROOT]/phpbb/event/dispatcher.php
LINE: 62
CALL: Symfony\Component\EventDispatcher\EventDispatcher->dispatch()

FILE: [ROOT]/phpbb/event/dispatcher.php
LINE: 46
CALL: phpbb\event\dispatcher->dispatch()

FILE: [ROOT]/includes/functions.php
LINE: 4244
CALL: phpbb\event\dispatcher->trigger_event()

FILE: [ROOT]/index.php
LINE: 252
CALL: page_footer()

Datenbank-Server: MySQL(i) 10.3.22-MariaDB-1:10.3.22+maria~stretch
phpBB Versiom 3.3.1 mit PHP 7.4.2

Wenn ich in der listener.php (Zeile 185 und 213) diese beiden Codestellen:

Code: Alles auswählen

' . $this->db->sql_escape($excluded_ids) . '
' . $this->db->sql_escape($excluded_ids_hours) . '

durch jenes ersetze:

Code: Alles auswählen

' . $excluded_ids . '
' . $excluded_ids_hours . '

funktioniert es wieder, aber dann meckert der Ext Check von LukeWCS (Found potential SQL injection) wieder.
Die komplette listener kann man in Pastebin ansehen.
Wo liegt der Fehler?

[Dr.Death]

Blind ins Blaue geschossen:

Stückel die SQL Abfrage testweise mal

Code: Alles auswählen

// $excluded_ids = (sizeof($exclude_ids_ary)) ? 'AND ' . $this->db->sql_in_set('user_id', $exclude_ids_ary, true) : '';

$sql = 'SELECT username, user_id, user_type, user_colour, user_posts
	FROM ' . USERS_TABLE . '
	WHERE user_id <> ' . (int) ANONYMOUS . '
		AND user_type <> ' . (int) USER_IGNORE . '
		AND user_posts > 0';
					
if (sizeof($exclude_ids_ary))
{
    $sql .= ' AND ' . $this->db->sql_in_set('user_id', $exclude_ids_ary, true);
}
	
$sql .= ' ORDER BY user_posts DESC';

[Kirk]

Danke, die 2. SQL Abfrage hatte ich von:

Code: Alles auswählen

			$sql = 'SELECT u.user_id, u.username, u.user_type, u.user_colour, COUNT(p.post_id) as total_posts
					FROM ' . USERS_TABLE . ' u, ' . POSTS_TABLE . ' p
					WHERE p.post_time > ' . (int) $time . '
						AND u.user_id = p.poster_id
							AND u.user_id <> ' . (int) ANONYMOUS . '
								AND u.user_type <> ' . (int) USER_IGNORE . '
									' . $this->db->sql_escape($excluded_ids_hours) . '
					GROUP BY u.user_id
					ORDER BY total_posts DESC';

so abgändert:

Code: Alles auswählen

		//$excluded_ids_hours = (sizeof($exclude_ids_ary_hours)) ? 'AND ' . $this->db->sql_in_set('user_id', $exclude_ids_ary_hours, true) : '';

			$sql = 'SELECT u.user_id, u.username, u.user_type, u.user_colour, COUNT(p.post_id) as total_posts
					FROM ' . USERS_TABLE . ' u, ' . POSTS_TABLE . ' p
					WHERE p.post_time > ' . (int) $time . '
						AND u.user_id = p.poster_id
							AND u.user_id <> ' . (int) ANONYMOUS . '
								AND u.user_type <> ' . (int) USER_IGNORE . '
						if (sizeof($exclude_ids_ary_hours))
						{
							$sql .= ' AND ' . $this->db->sql_in_set('user_id', $exclude_ids_ary_hours, true);
						}
					$sql .= GROUP BY u.user_id';
					$sql .= ' ORDER BY total_posts DESC';

das passt aber noch nicht, denn ich bekomme folgende Fehlermeldung:

Parse error: syntax error, unexpected 'user_id' (T_STRING) in /ext/kirk/topposters/event/listener.php on line 218

Irgendwo darin ist ein Zeichen verkehrt, ich komm nicht dahinter wo.

[Dr.Death]

Code: Alles auswählen

			$sql = 'SELECT u.user_id, u.username, u.user_type, u.user_colour, COUNT(p.post_id) as total_posts
					FROM ' . USERS_TABLE . ' u, ' . POSTS_TABLE . ' p
					WHERE p.post_time > ' . (int) $time . '
						AND u.user_id = p.poster_id
						AND u.user_id <> ' . (int) ANONYMOUS . '
						AND u.user_type <> ' . (int) USER_IGNORE;
						
			if (sizeof($exclude_ids_ary_hours))
			{
				$sql .= ' AND ' . $this->db->sql_in_set('user_id', $exclude_ids_ary_hours, true);
			}
			
			$sql .= ' GROUP BY u.user_id';
			$sql .= ' ORDER BY total_posts DESC';

[Kirk]

Besten Dank Doc, ich hatte immer wieder an der verkehrten Stelle etwas geändert, kaum macht man`s richtig funktioniert es und der Ext Check hat an denen Stellen auch nicht zu bemängeln.

[Dr.Death]

Perfekt, danke für die Rückmeldung.

Hatte das nur so „trocken“ hingeschrieben ohne zu testen. Freut mich das es so funktioniert hat.