Информация Мини-аудит кода

Горбушка · 26 Мар 2017

Нет, я не собираюсь проверять ДЛЕ на уязвимости - для этого есть другие ресурсы... И они довольно бодро этим занимаются. Меня интересует откровенная дурость в коде.

В этой теме буду выкладывать куски кода, которых надеюсь не увидеть в следующих версиях.

Горбушка · 26 Мар 2017

DataLife Engine 11.2 utf-8
/engine/inc/googlemap.php

PHP:

if( !$user_group[$member_id['user_group']]['admin_googlemap'] ) {
   msg( "error", $lang['index_denied'], $lang['index_denied'] );
}

//################# Определение групп пользователей
$user_group = get_vars ( "usergroup" );

if (!is_array( $user_group )) {
   $user_group = array ();

   $db->query ( "SELECT * FROM " . USERPREFIX . "_usergroups ORDER BY id ASC" );

   while ( $row = $db->get_row () ) {

      $user_group[$row['id']] = array ();

      foreach ( $row as $key => $value ) {
         $user_group[$row['id']][$key] = stripslashes($value);
      }

   }
   set_vars ( "usergroup", $user_group );
   $db->free ();
}

Может я чего не понимаю, но мы сначала проверяем значение $user_group, а 3 строками ниже мы получаем её...

Нужно актуализировать? Чтобы взять не из кэша? Ну-ну... Поэтому мы её из кэша и получаем.
Спрашивается, чем не устроил этот же блок кода в /engine/inc/include/init.php * ? Отличается он только проверкой: if(!is_array($user_group)) против if(!$user_group) в init.php

Я бы понял, если бы оно нужно было для cron и т.д., но этот блок кода выполняется ТОЛЬКО если $user_group уже существует.

Не вижу ничего, кроме лишний нагрузки и дублировании кода.

* - файл зашифрован. Согласно закону, декодирование файлов с целью изучения без в мешательства в механизмы защиты разрешено. Статья 1280 ГК РФ.

Gameer · 27 Мар 2017

Все это конечно хорошо, но так можно докопаться к любому коду.
К примеру в твоем коде

PHP:

elseif ($action == 'delete') {

    include(ENGINE_DIR . '/inc/g_callback/delete.php');

}

идет подключение файла в котором код

PHP:

<?php

// Anti-hotlink
if( ! defined( 'DATALIFEENGINE' ) OR !defined( 'LOGGED_IN' ) ) {
    die( "Hacking attempt!" );
}

echo g_delete_request();

Зачем подключать не нужный код с вызовом единой функции и не нужной проверкой когда это можно сделать вместо того же подключения файла, ведь там уже есть проверка на define...

Горбушка · 27 Мар 2017

Гамеер, Гамеер... Когда ж ты уже повзрослеешь как программист?

Докопаться можно ко всему, но отвечу...
1) Очень классно сравнивать бесплатной модуль даже не в Альфе с промышленной системой
2) Есть такое понятие, как единообразие кода - если edit, view и т.д. сделаны отдельными файлами - то и delete должен быть таким же, пусть и в 1 строку
3) Что лишнего в коже? Hotlink защита? Ну ок, убери, при прямом обращении к файлу получишь ошибку PHP с раскрытием путей

Вот если бы я вызывал g_delete_request(); в основном файле, а потом делал include и ещё раз g_delete_request(); - вот это было бы то, что описано мной выше.

И вообще, а на кой DLE разброшен на 100500 файлов? Даёшь всё в index.php - меньше обращений к HDD - круто!

Gameer · 27 Мар 2017

Горбушка написал(а):
Гамеер, Гамеер... Когда ж ты уже повзрослеешь как программист?

Докопаться можно ко всему, но отвечу...
1) Очень классно сравнивать бесплатной модуль даже не в Альфе с промышленной системой
2) Есть такое понятие, как единообразие кода - если edit, view и т.д. сделаны отдельными файлами - то и delete должен быть таким же, пусть и в 1 строку
3) Что лишнего в коже? Hotlink защита? Ну ок, убери, при прямом обращении к файлу получишь ошибку PHP с раскрытием путей

Вот если бы я вызывал g_delete_request(); в основном файле, а потом делал include и ещё раз g_delete_request(); - вот это было бы то, что описано мной выше.

И вообще, а на кой DLE разброшен на 100500 файлов? Даёшь всё в index.php - меньше обращений к HDD - круто!

Когда ж ты уже повзрослеешь как программист? Я свою мысль выложил в сообщении, что можно докопаться до чего угодно, даже до табуляции. Вместо того что бы анализировать чужой код, допиши тот же бесплатный модуль который даже не в Альфе (хотя, если модуль бесплатный это позволяет писать код как угодно? Тогда может стоит дописать до альфы и тогда выпускать? Кто знает...). Я не писал что в DLE нормальный код, нет, он там ужасный. На сим откланяюсь, думаю сей мини конфликт исчерпан и сообщения не будут удалены (как обычно).

PirateGod · 28 Мар 2017

Может быть имеет смысл писать всё в обратную связь DLE или на форуме DLE в багах. Тут то для кого?

Горбушка · 28 Мар 2017

PirateGod написал(а):
Может быть имеет смысл писать всё в обратную связь DLE или на форуме DLE в багах. Тут то для кого?

Ты не поверишь, за нами следят

Горбушка · 27 Ноя 2017

Наткнулся я ещё на один ну просто идиотический момент в DLE... На этот раз - панель администратора...

Понадобилось мне поменять в панели администратора кнопочки в шапке. Казалось бы, рядовая задача, но в DLE она решается очень сложно.
Хотя давайте скажем правду, в модулях, даже в штатных самого DLE, ссылки на редактирование новостей - не самая актуальная.

Давайте посмотрим на структуру файлов, которые отвечают за панель администратора.
/engine/skins/default.skin.php - файл шаблона панели администратора. Точнее, не всего шаблона, а его "шапки" и "подвала". Он же отвечает за страницу входа в панель администратора, формирование списка модулей слева, аватарки и информации о пользователе в "шапке", а так же количества личных сообщений.
/engine/inc/include/functions.inc.php - файл основных функций, в том числе и вывода "шапки" и "подвала" сайта...

Что ж, давайте рассмотрим возможные варианты, которые помогут нам заменить или дополнить ссылки в "шапке".

Первое, что приходит в голову, так это динамически генерируемый список ссылок. Давайте проверим:

Скрытое содержимое доступно для зарегистрированных пользователей!

Увы, нет, кусок записан жёстко и не выводится переменными. Можно, конечно, подменить переменные $lang['add_news'] и $lang['edit_news'], но это не правильно и может отразиться на других участках кода.
Подмена переменных - это вообще крайняя мера в программировании на мой взгляд.

Второй вариант, который приходит в голову - это возможность выбора шаблона и замена шаблона по-умолчанию на наш. Звучит очень даже правдоподобно, учитывая, что наш шаблон называется "default". Где есть шаблон "по-умолчанию", там должны быть и другие...
Смотрим функцию, которая выводит шаблон на экран:

Скрытое содержимое доступно для зарегистрированных пользователей!

К сожалению, и здесь нам нечего искать. Никаких условий или переменных, отвечающих за подключение шаблона нет. Единственное, что реализовывает функция - генерация "хлебных крошек" или "спидбара". Это панелька навигации, показывающая в каком разделе Вы сейчас находитесь.

Что ж, DLE на дал нам ни единого варианта, чтобы управлять шаблоном, "шапкой" и т.д. Будем исправлять.

Самый тупой вариант, который приходит в голову - это вкорячить изменения прямиком в файл default.skin.php. Нет, это явно плохая идея, которая приведёт к проблемам с обновлениями. Да и отразится на других участках панели адинистратора.
Вариант лучше - это скорректировать файл functions.inc.php, добавив в него возможность выбора шаблона. К примеру так:

Скрытое содержимое доступно для зарегистрированных пользователей!

В результате, мы получаем в любом куске кода DLE выбор шаблона default.skin.php, но достаточно передать 3-ю переменную - мы подгружаем другой шаблон.
Именно это решение я бы рекомендовал внедрить разработчику DLE, чтобы сильно облегчить работу разработчиков дополнений. Да и решение напрашивается само собой. Думаю, это даже изначально было задумано разработчиком, исходя из структуры папок и файлов, но почему-то не было реализовано.

Но мы - разработчики дополнений, и наша задача - делать модули максимально независимыми от основного движка, чтобы обновление DLE не потребовало новой установки модуля. Хотя это и не возможно полностью - в пользовательской части всё равно придётся править файлы, но минимизировать это необходимо. Хотя опять же, остаётся вопросом - почему в панели администратора реализовано автоматическое подключение модулей, а в пользовательской части - нет.

Но это просто разговор ни о чём, давайте всё же реализовывать задуманное.

Первым делом мы скопируем default.skin.php и сохраним его с именем gorbushka.skin.php (да, я тщеславен). Дальше вносим изменения в файл, которые мы хотели сделать. Меняем ссылки, добавляем новые переменные и т.д. Думаю, с этим справитесь.

Теперь нам надо переопределить функцию echoheader(). Создадим свою функцию и назовём _echoheader() - напомню, переопределять переменные и функции плохая идея. В теле самой функции можем как просто прописать подключение другого шаблона, так и реализовать выбор шаблона, как я писал выше. Это дело вкуса. Остальной текст функции мы полностью копируем из исходной echoheader().

Теперь в своём модуле мы просто вызываем новую функцию _echoheader() вместо echoheader().

Gameer · 27 Ноя 2017

Горбушка написал(а):
Но мы - разработчики дополнений, и наша задача - делать модули максимально независимыми от основного движка, чтобы обновление DLE не потребовало новой установки модуля. Хотя это и не возможно полностью - в пользовательской части всё равно придётся править файлы, но минимизировать это необходимо. Хотя опять же, остаётся вопросом - почему в панели администратора реализовано автоматическое подключение модулей, а в пользовательской части - нет.

Решение простое - отдельный дизайн админ панели от основной DLE. Потому что она меняется уже с каждым обновлением.

Теперь я прикопаюсь к коду DLE.
Быстрый поиск на сайте имеет ошибку в DLE на всех версиях. Если в названии новости есть одинарные или двойные кавычки быстрый поиск эту новость не найдет, НО, полный поиск - находит. А вся проблема в том, что реализация обработки поискового запроса в быстром поиске и в поиске на отдельной странице - РАЗНЫЙ. Так в файле /engine/modules/search.php есть функция, которая почему-то не вынесена в файл /engine/modules/functions.php

PHP:

function strip_data($text) {

    $quotes = array ( "\x60", "\t", "\n", "\r", ",", ";", ":", "[", "]", "{", "}", "=", "*", "^", "%", "$", "<", ">", "+", "-" );
    $goodquotes = array ("#", "'", '"', "&" );
    $repquotes = array ("\#", "\'", '\"', "&amp;" );
    $text = stripslashes( $text );
    $text = trim( strip_tags( $text ) );
    $text = str_replace( $quotes, '', $text );
    $text = str_replace( $goodquotes, $repquotes, $text );
   
    return $text;
}

А вот обработка самого текста

PHP:

if( isset( $_REQUEST['story'] ) ) $story = dle_substr( strip_data( rawurldecode( $_REQUEST['story'] ) ), 0, 90, $config['charset'] ); else $story = "";

if( function_exists( "get_magic_quotes_gpc" ) && get_magic_quotes_gpc() ) $story = stripslashes( $story );

$story = addslashes( $story );

$story = $db->safesql($story);

В то время как в файле /engine/ajax/search.php

PHP:

$query = $db->safesql( htmlspecialchars ( trim(  strip_tags (convert_unicode( $_POST['query'], $config['charset'] ) ) ), ENT_QUOTES, $config['charset']) );

Потому что в /engine/ajax/search.php идет преобразование в html сущности...

В то время как в /engine/modules/search.php идет экранирование

А все дело в том что код быстрого поиска не менялся за часов еще 9.0. Только внедряли функции которые добавлялись в новых версиях, попутно забывая что его самого нужно обновить.

Горбушка · 27 Ноя 2017

Gameer написал(а):
Решение простое - отдельный дизайн админ панели от основной DLE. Потому что она меняется уже с каждым обновлением.

Глобально - да, я с этим согласен. Это наиболее простое решение и защищает от изменений в DLE. По хорошему, использовать свой класс работы с БД, шаблонизатор и многое другое. Тогда модуль встанет вообще на любую версию DLE.
Увы, это не очень красиво для пользователей. Я стараюсь всегда делать максимальную интеграцию с исходной системой, чтобы не было видно, что ты перешёл в сторонний функционал. Всё же, мы делаем модуль, а не интеграцию с внешней системой.

Информация Мини-аудит кода

Горбушка

Ищу её...

Горбушка

Ищу её...

Gameer

Постоялец

Горбушка

Ищу её...

Gameer

Постоялец

PirateGod

Постоялец

Горбушка

Ищу её...

Горбушка

Ищу её...

Gameer

Постоялец

Горбушка

Ищу её...