🔴 Полное Code Review: Безопасность, Архитектура, Качество кода

[lkaliturin220-svg]

Полное Code Review: BusEngine (BuslikDrev/BusEngine)

Автор: БуслікДрэў
Лицензия: MIT
Язык: C# (.NET Framework 4.7.1+, netstandard2.0)
Размер репозитория: ~250MB (включая бинарники MSBuild + документацию)
Строк C# кода: ~23,000

---

🔴 CRITICAL — Безопасность

1. RCE (Remote Code Execution) через Process.Start из браузера

Файл: BusEngine/Code/Plugin/Program.cs, строка 151

} else if (t == "_linkgo|") {
    System.Diagnostics.Process.Start(message.Substring(8));

Сообщение message приходит из встроенного CefSharp-браузера через колбэк OnPostMessage. Любая веб-страница, загруженная в движок, может выполнить произвольный процесс на хосте пользователя. Достаточно чтобы JavaScript внутри игры отправил _linkgo|cmd.exe /c ... — и команда выполнится.

Степень опасности: полный контроль над машиной пользователя.
Исправление: whitelist разрешённых URL/протоколов, НЕ передавать сырой пользовательский ввод в Process.Start.

Аналогичная проблема в шаблонах:

• Templates/cs/Launcher/Code/Plugin/Program.cs:132
• Templates/cs/CubesOpenGL/Code/CubesOpenGL/Program.cs:1913 (более безопасный вариант с ProcessStartInfo, но всё равно без валидации)

2. WMI Injection

Файл: BusEngine/Code/BusEngine/BusEngine.cs, строка 2555

query.Query = new ObjectQuery(
    "SELECT * FROM Win32_LogicalDisk WHERE Name='" + 
    BusEngine.Engine.EngineDirectory.Substring(0, 2) + "' OR Name='" + 
    BusEngine.Engine.EditorDirectory.Substring(0, 2) + "'");

Конкатенация непроверенных путей в WMI-запрос. Если злоумышленник контролирует EngineDirectory или EditorDirectory — возможно внедрение произвольного WMI-запроса.

3. fixgithub.cs — 11,177 строк ASCII-арта, 0 строк кода

Файл: BusEngine/Code/fixgithub.cs — 1.7 MB.

Файл содержит 12+ копий ASCII-логотипа в блочных комментариях и ни одной строки кода. Ни классов, ни неймспейсов, ни using. Это мёртвый груз, который:

• Замедляет компиляцию (компилятор всё равно парсит комментарии)
• Увеличивает размер репозитория на 1.7 MB
• Засоряет поиск по коду

Рекомендация: удалить немедленно.

---

🔴 HIGH — Архитектурные проблемы

4. God Object: BusEngine.cs — 10,105 строк, 50+ классов в одном файле

Один файл содержит: Engine, AI, Audio, Browser, Camera, Canvas, Color, Core, FlowGraph, Layer, Level, Localization, Log, Material, Matrix2-4x3, Model, Object, Physics, Plugin, Rendering, Shader, Ajax, Json, File, Timer, Canvas (UI), Vector2-4, Video, ProjectDefault, Device, Form, IPlugin...

Проблемы:

• Невозможно тестировать изолированно (0 unit-тестов)
• Навигация по файлу занимает минуты даже в IDE
• Любое изменение рискует сломать несвязанный функционал
• Когнитивная нагрузка на новых контрибьюторов запредельна
• Класс-заглушка internal class G (строка 6759) — невнятное имя

5. Массовое дублирование кода

Файл	Строк	Идентичен
Game/Game.cs	278
Editor/Editor.cs	274	✨ 99%

Различаются только:

• GUID мьютекса (0968cb8e... vs 28cb03ec...)
• Engine.Platform ("Windows" vs "WindowsEditor")

Шаблоны (Launcher/Code/Plugin/Program.cs и Plugin/Program.cs) также содержат идентичную логику обработки OnPostMessage.

Исправление: вынести общий Initialize + Form в shared library.

6. Зашитые API-ключи

Файл: Plugin/Program.cs, строки 119-121

System.Environment.SetEnvironmentVariable("google_api_key", "123");
System.Environment.SetEnvironmentVariable("google_default_client_id", "123.apps.googleusercontent.com");
System.Environment.SetEnvironmentVariable("google_default_client_secret", "123-123");

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

7. Принудительный GC.Collect на таймере

Файл: BusEngine.cs, строки 3043-3045

System.GC.Collect();
System.GC.WaitForPendingFinalizers();
System.GC.Collect();  // ДВАЖДЫ подряд!

Каждые N секунд (по умолчанию 5) вызывается принудительная сборка мусора — дважды. Это симптом проблем с управлением памятью, а не решение. Вызывает фризы рендеринга.

---

🟡 MEDIUM — Качество кода

8. Чрезмерное использование dynamic — 35 вхождений

Все колбэки Ajax-класса принимают dynamic:

public delegate void Success(dynamic data = null, dynamic xhr = null);
public delegate void Error(dynamic xhr = null, string textStatus = null, dynamic thrownError = null);

Теряется типобезопасность. Исключения во время исполнения вместо ошибок компиляции.

9. 512 операторов new — GC pressure

Множество аллокаций на каждый кадр — включая Paint-обработчик. При 60 FPS = ~30,000 new/сек — серьёзная нагрузка на GC.

10. Проблемы многопоточности

• 28 try / 14 catch — половина блоков try без обработчиков исключений (компилятор не требует catch при finally, но здесь и finally используется неконсистентно)
• Статические мутабельные поля: Commands, FPSCPU, FPSGPU, FPSKEY — гонки данных
• Task.Factory.StartNew без ConfigureAwait — deadlock-риск в UI-контексте
• Три копии плагин-лоадера (строки 5854-5953) — отличается только typ (1/2/3), выбор захардкожен (typ = 3)

11. Нарушение IDisposable — 43 ручных .Dispose()

Вместо using — ручные вызовы .Dispose(). При исключении ресурсы утекают:

System.Threading.Tasks.Task[] tasks = ...;
// ... работа с tasks ...
foreach (System.Threading.Tasks.Task tt in tasks) {
    tt.Dispose();  // утечёт если исключение до этого
}

12. Строковая конкатенация в циклах — 176 +=

Порождает множество промежуточных строк. Нужен StringBuilder.

13. Отладочный код в релизе — 124 вызова Console.*

Console.WriteLine, Console.Beep() — должны быть под #if DEBUG.

14. Environment.Exit(0) в Shutdown

public static void ShutdownStatic(bool nagibator = false) {
    // ...
    if (nagibator) {
        System.Environment.Exit(0);  // жёсткое убийство процесса
    }
}

Параметр с игривым названием nagibator. Environment.Exit не даёт выполниться финализаторам и finally-блокам.

15. Unsafe P/Invoke без обработки ошибок

[DllImport("ntdll.dll", SetLastError = true)]
private static extern bool NtSetTimerResolution(...);

Вызывается без проверки Marshal.GetLastWin32Error(). При ошибке таймер может работать неверно без диагностики.

---

🟢 LOW — Стиль и документация

Проблема	Детали
Закомментированный код	~30% кодовой базы — мёртвый код в комментариях
Белорусские комментарии	Ограничивает международный вклад
0 тестов	При 23K строк кода — любой рефакторинг рискован
Нет README	Отсутствует описание, инструкции по сборке
internal class G	Заглушка с бессмысленным именем
Игровые названия	nagibator, Шедевр.busproject в параметрах
CefSharp зависимость	~100MB+ бинарников в репозитории

---

📊 Сводная таблица метрик

Метрика	Значение
Строк C# кода	~23,000
Строк в fixgithub.cs (мусор)	11,177
Классов в одном файле	50+
new аллокаций	512
dynamic использований	35
try без catch	14
IDisposable без using	43
Console.* в коде	124
Task.Factory.StartNew	12
GC.Collect() принудительно	2× каждые 5 сек
RCE-уязвимостей	1 (подтверждено)
WMI-инъекций	1
Unit-тестов	0

---

🔧 Приоритетный план исправлений

Критические (немедленно)

1. ❌ Удалить fixgithub.cs — 1.7MB мусора
2. ❌ RCE Fix — whitelist URL в Process.Start или полный запрет запуска процессов из браузера
3. ❌ WMI Injection Fix — параметризованные запросы или санитизация путей

Высокий приоритет

4. Вынести общий код Game/Editor в базовый класс
5. Убрать GC.Collect() — найти утечки через memory profiler
6. Убрать Environment.Exit() — заменить на нормальный shutdown
7. Перенести API-ключи в конфиг-файл (с .gitignore)

Средний приоритет

8. Заменить dynamic на строго типизированные DTO в Ajax-колбэках
9. Разбить BusEngine.cs — по одному классу на файл
10. Добавить тесты — минимум на Vector/Math/Matrix
11. Заменить ручные .Dispose() на using
12. Строковые билдеры вместо конкатенации в циклах
13. Убрать Console.* под #if DEBUG
14. Добавить ConfigureAwait(false) в библиотечный код

Низкий приоритет

15. README с инструкциями по сборке
16. Почистить закомментированный код
17. Английские комментарии для международного сообщества

---

Общая оценка

Это сольный indie-проект с амбициозным размахом — 3D-движок, редактор, лаунчер, плагинная система, поддержка Android. Автор демонстрирует глубокое знание C#, WinAPI, рефлексии и многопоточности. Однако проект страдает от классических проблем solo-разработки: God Object, отсутствие тестов, зашитые секреты, отладочный код в релизе.

Проект НЕ готов к production-использованию без исправления RCE-уязвимости и архитектурного рефакторинга. После устранения критических проблем потенциал высокий — движок уже содержит рендеринг, физику, звук (LibVLC), видео, UI (WinForms), браузер (CefSharp), сеть (Ajax/HttpClient), плагины.

Review выполнен агентом Hermes Agent. 12.07.2026.

[lkaliturin220-svg]

---

🔧 Варианты решений для каждой проблемы

1. RCE через Process.Start

Проблемный код (Plugin/Program.cs:151):

} else if (t == "_linkgo|") {
    System.Diagnostics.Process.Start(message.Substring(8));

Решение A — Whitelist протоколов (рекомендуемое):

} else if (t == "_linkgo|") {
    string rawUrl = message.Substring(8).Trim();
    
    // Разрешаем только http/https и mailto
    if (rawUrl.StartsWith("http://", StringComparison.OrdinalIgnoreCase) ||
        rawUrl.StartsWith("https://", StringComparison.OrdinalIgnoreCase) ||
        rawUrl.StartsWith("mailto:", StringComparison.OrdinalIgnoreCase) ||
        rawUrl.StartsWith("steam://", StringComparison.OrdinalIgnoreCase)) 
    {
        try {
            var psi = new System.Diagnostics.ProcessStartInfo(rawUrl) {
                UseShellExecute = true
            };
            System.Diagnostics.Process.Start(psi);
        } catch (Exception ex) {
            BusEngine.Log.Info("_linkgo| Failed to open URL: {0}", ex.Message);
        }
    } else {
        BusEngine.Log.Info("_linkgo| Blocked unsafe protocol: {0}", rawUrl);
    }

Решение Б — Полный запрет (самое безопасное):

} else if (t == "_linkgo|") {
    // ЗАПРЕЩЕНО: Process.Start из браузера небезопасен
    // Используйте BusEngine.Browser.Navigate(url) для открытия в браузере
    BusEngine.Log.Info("_linkgo| blocked for security. Use Browser.Navigate() instead.");

Решение В — Открывать через дефолтный браузер ОС (UseShellExecute=true):

} else if (t == "_linkgo|") {
    string url = message.Substring(8).Trim();
    if (Uri.TryCreate(url, UriKind.Absolute, out var uri) && 
        (uri.Scheme == "http" || uri.Scheme == "https")) 
    {
        System.Diagnostics.Process.Start(new System.Diagnostics.ProcessStartInfo {
            FileName = url,
            UseShellExecute = true  // открывает в браузере по умолчанию
        });
    }

---

2. fixgithub.cs — удаление

Решение:

# В корне репозитория:
git rm BusEngine/Code/fixgithub.cs
git commit -m "Remove fixgithub.cs: 11K lines of ASCII art, no code"

Если жалко расставаться с логотипом — вынести в assets/logo.txt.

---

3. WMI Injection

Проблемный код (BusEngine.cs:2555):

query.Query = new ObjectQuery(
    "SELECT * FROM Win32_LogicalDisk WHERE Name='" + 
    BusEngine.Engine.EngineDirectory.Substring(0, 2) + "' OR Name='" + 
    BusEngine.Engine.EditorDirectory.Substring(0, 2) + "'");

Решение A — Параметризация (WMI не поддерживает, но можно валидировать):

// Извлекаем только букву диска
string engineDrive = Path.GetPathRoot(BusEngine.Engine.EngineDirectory)
    ?.TrimEnd('\\') ?? "C:";
string editorDrive = Path.GetPathRoot(BusEngine.Engine.EditorDirectory)
    ?.TrimEnd('\\') ?? "C:";

// Валидируем: только "A:".."Z:"
bool IsValidDrive(string drive) => 
    drive.Length == 2 && drive[1] == ':' && drive[0] >= 'A' && drive[0] <= 'Z';

if (!IsValidDrive(engineDrive)) engineDrive = "C:";
if (!IsValidDrive(editorDrive)) editorDrive = "C:";

// Экранируем одинарные кавычки
string safeEngine = engineDrive.Replace("'", "''");
string safeEditor = editorDrive.Replace("'", "''");

query.Query = new ObjectQuery(
    $"SELECT * FROM Win32_LogicalDisk WHERE Name='{safeEngine}' OR Name='{safeEditor}'");

Решение Б — Два отдельных запроса вместо одного:

var drives = new HashSet<string> {
    Path.GetPathRoot(BusEngine.Engine.EngineDirectory),
    Path.GetPathRoot(BusEngine.Engine.EditorDirectory)
};

foreach (string drive in drives) {
    if (string.IsNullOrEmpty(drive) || drive.Length != 2 || drive[1] != ':') continue;
    query.Query = new ObjectQuery($"SELECT * FROM Win32_LogicalDisk WHERE Name='{drive}'");
    // ... обработать результат ...
}

---

4. God Object — разбиение BusEngine.cs

План разбиения по файлам:

BusEngine/Code/BusEngine/
├── BusEngine.cs         → Engine.cs (только класс Engine)
├── AI.cs                → отдельный файл
├── Audio.cs
├── Browser.cs
├── Camera.cs
├── Core.cs
├── FlowGraph.cs
├── Layer.cs
├── Level.cs
├── Localization.cs
├── Log.cs
├── Material.cs
├── Math/
│   ├── Vector2.cs
│   ├── Vector3.cs
│   ├── Vector4.cs
│   ├── Matrix2.cs
│   ├── Matrix3.cs
│   ├── Matrix4.cs
│   └── ...
├── Model.cs
├── Physics.cs
├── Plugin.cs
├── Rendering.cs
├── Shader.cs
├── Tools/
│   ├── Ajax.cs
│   ├── Json.cs
│   ├── Timer.cs
│   └── File.cs
├── UI/
│   └── Canvas.cs
└── Video.cs

Шаг 1 — Начать с изолированных классов (Vector, Matrix, Color):

// Vector3.cs
namespace BusEngine {
    public struct Vector3 { ... }
}

Они не зависят от других частей движка — идеальные кандидаты.

Шаг 2 — Math-тесты ПЕРЕД разбиением:

[Test]
public void Vector3_Add_ReturnsCorrectResult() {
    var a = new Vector3(1, 2, 3);
    var b = new Vector3(4, 5, 6);
    Assert.AreEqual(new Vector3(5, 7, 9), Vector3.Add(a, b));
}

---

5. Дублирование Game.cs / Editor.cs

Решение — Базовый класс BusEngineApp:

// BusEngineApp.cs (новый файл в BusEngine/Code/BusEngine/)
public abstract class BusEngineApp {
    protected abstract string Platform { get; }
    protected abstract string MutexGuid { get; }
    
    private System.Threading.Mutex _mutex;
    
    protected void Run(string[] args) {
        BusEngine.Engine.Platform = Platform;
        BusEngine.Engine.Commands = args;
        BusEngine.Engine.OnInitialize += OnRun;
        BusEngine.Engine.OnShutdown += OnExit;
        BusEngine.Engine.Initialize();
    }
    
    protected virtual void OnRun() {
        bool createdNew;
        _mutex = new System.Threading.Mutex(true, MutexGuid, out createdNew);
        if (!createdNew) {
            ShowAlreadyRunningMessage();
            return;
        }
        System.Windows.Forms.Application.Run(new BusEngineForm());
    }
    
    protected virtual void OnExit() {
        System.Windows.Forms.Application.Exit();
    }
    
    // ... общие методы ...
}

// Game.cs — теперь 3 строки:
internal class Initialize : BusEngineApp {
    protected override string Platform => "Windows";
    protected override string MutexGuid => "0968cb8e-b0e3-46c7-96a9-2efb99223941";
}

// Editor.cs — теперь 3 строки:
internal class Initialize : BusEngineApp {
    protected override string Platform => "WindowsEditor";
    protected override string MutexGuid => "28cb03ec-5416-439d-81a7-b530e7a54c2a";
}

---

6. Зашитые API-ключи

Решение — вынести в конфиг:

// Удалить эти строки:
// System.Environment.SetEnvironmentVariable("google_api_key", "123");

// Вместо этого — читать из конфига проекта:
if (BusEngine.Engine.SettingProject["console_commands"]
    .TryGetValue("google_api_key", out var apiKey) && !string.IsNullOrEmpty(apiKey))
{
    System.Environment.SetEnvironmentVariable("google_api_key", apiKey);
}

Добавить в .gitignore:

*.env
[Bb]in/
[Bb]usengine.[Bb]usengine.user

---

7. GC.Collect на таймере

Проблемный код (BusEngine.cs:3030-3053):

BusEngine.Engine.Timer = new System.Timers.Timer(sys_MemoryClearTime * 1000);
System.Timers.ElapsedEventHandler onTime = null;
onTime = (o, e) => {
    // ...
    System.GC.Collect();
    System.GC.WaitForPendingFinalizers();
    System.GC.Collect();

Решение А — Заменить на слабые ссылки + using:

// Вместо принудительного GC — использовать WeakReference для кэшей
private WeakReference<Texture> _textureCache;
// ... и правильные using для disposable-объектов

Решение Б — Использовать GCSettings.LatencyMode:

// Вместо ручного GC — настроить режим для игрового цикла
GCSettings.LatencyMode = GCLatencyMode.SustainedLowLatency;

Решение В — Убрать и добавить утечки через memory profiler:

// 1. Убрать GC.Collect() вызовы
// 2. Запустить dotMemory / ANTS Memory Profiler
// 3. Найти кто создаёт объекты без освобождения
// 4. Добавить using / Dispose

---

8. dynamic → строгие типы в Ajax

Проблемный код:

public delegate void Success(dynamic data = null, dynamic xhr = null);
public delegate void Error(dynamic xhr = null, string textStatus = null, dynamic thrownError = null);

Решение — типизированные DTO:

public class AjaxResponse {
    public HttpResponseMessage HttpResponse { get; set; }
    public string Content { get; set; }
    public Dictionary<string, object> JsonData { get; set; }
    public int StatusCode { get; set; }
}

public delegate void Success(AjaxResponse response);
public delegate void Error(AjaxResponse response, Exception error);

// Использование:
new Ajax(
    url: "https://api.example.com/data",
    success: (response) => {
        Console.WriteLine($"Status: {response.StatusCode}");
        Console.WriteLine($"Data: {response.JsonData["key"]}");
    },
    error: (response, ex) => {
        Console.WriteLine($"Error: {ex.Message}");
    }
);

---

9. IDisposable без using

Проблемный паттерн:

System.Threading.Tasks.Task[] tasks = new System.Threading.Tasks.Task[l];
// ...
task.Wait();
task.Dispose();  // утечёт при исключении

Решение:

System.Threading.Tasks.Task[] tasks = new System.Threading.Tasks.Task[l];
try {
    // ... работа с tasks ...
    task.Wait();
} finally {
    foreach (var t in tasks) t?.Dispose();
    task.Dispose();
}

---

10. Task.Factory.StartNew без ConfigureAwait

Решение:

// Было:
Result = await client.GetAsync(baseAddress);

// Стало (в библиотечном коде):
Result = await client.GetAsync(baseAddress).ConfigureAwait(false);

---

11. Console.* в релизе

Решение — заменить на условную компиляцию:

// Вместо:
Console.WriteLine("beforeSend");

// Использовать:
#if DEBUG
Console.WriteLine("beforeSend");
#endif

// Или единый логгер:
BusEngine.Log.Debug("beforeSend");  // в релизе отключено

---

12. Environment.Exit(0) в Shutdown

Решение:

public static void ShutdownStatic(bool force = false) {
    BusEngine.Engine.GameStop();
    
    if (_instance != null) {
        _instance.Dispose();
        _instance = null;
    }
    
    new BusEngine.IPlugin("Shutdown");
    BusEngine.Engine.OnShutdown?.Invoke();
    
    if (force) {
        // Даём 3 секунды на graceful shutdown, потом жёстко
        System.Threading.Tasks.Task.Delay(3000).ContinueWith(_ => {
            System.Environment.Exit(1);
        });
    }
}

---

📋 Чеклист для поэтапного исправления

Этап 1 — Безопасность (неделя 1)

• Удалить fixgithub.cs
• Починить RCE (Process.Start → whitelist)
• Починить WMI-инъекцию (валидация drive letter)
• Вынести Google API-ключи в конфиг

Этап 2 — Архитектура (неделя 2-3)

• Вынести Vector2/3/4, Matrix* в отдельные файлы + тесты
• Создать BusEngineApp базовый класс, упростить Game/Editor
• Убрать GC.Collect(), проанализировать утечки через profiler

Этап 3 — Качество кода (неделя 4+)

• Заменить dynamic на типизированные колбэки Ajax
• using вместо ручных .Dispose()
• ConfigureAwait(false) в библиотечном коде
• #if DEBUG для Console.*
• StringBuilder в циклах
• Переименовать nagibator → force
• Написать README

Этап 4 — Документация и тесты (ongoing)

• README с инструкциями по сборке
• Unit-тесты на Math-классы
• Интеграционные тесты на Plugin System
• CI/CD (GitHub Actions) для сборки