Security hardening round 2: comment-aware forbidden-pattern check, UPDATE confirmation, transactional batches, quote-aware split (incl. double quotes), NULL-safe table rendering, AJAX timeout, request size limit

- removeAllComments() strips -- and /* */ comments before checking
  FORBIDDEN_PATTERNS, closing a bypass where a comment inserted between
  a function name and '(' hid it from the regex (e.g. pg_read_file/**/(...)).
- UPDATE now requires confirm=true just like DELETE (deleteCount/updateCount).
- Batches run inside a single DB transaction; a failing statement rolls back
  everything already applied in that batch instead of leaving partial writes.
- splitStatements() (PHP and JS) now tracks a single quoteChar instead of a
  bool, handling both '...' strings and "..." quoted identifiers symmetrically.
- Frontend TEMPLATES no longer rely on SET/current_setting: the escaped uid
  is inlined into every statement, so partial copy/paste still works.
- renderTable() renders NULL as [NULL] instead of an empty string, so it's
  no longer visually identical to an empty string value.
- Added MAX_SQL_BYTES (1MB) request size limit, returned as HTTP 413.
- Added a 120s client-side AJAX timeout with a distinct timeout message,
  so a hung request doesn't leave the terminal paused indefinitely.

js/index.js

@@ -14,26 +14,34 @@
     var OCC_PROMPT = 'occ $ ';
     var SQL_PROMPT = '[[;#ff5555;]sql]# ';
 
-    // Разбивает пачку запросов по ";" с учётом одинарных кавычек (в т.ч. ''
+    // Разбивает пачку запросов по ";" с учётом одинарных И двойных кавычек
-    // как экранированной кавычки внутри строки), чтобы ";" в строковом
+    // (в т.ч. '' / "" как экранированной кавычки внутри строки/идентификатора),
-    // литерале не ломал разбиение. Зеркалит splitStatements() на бэкенде.
+    // чтобы ";" в строковом литерале или "квотированном идентификаторе" не
+    // ломал разбиение. Зеркалит splitStatements() на бэкенде.
     function splitStatements(sql) {
       var statements = [];
       var current = '';
-      var inString = false;
+      var quoteChar = null;
       for (var i = 0; i < sql.length; i++) {
         var ch = sql[i];
-        if (ch === "'") {
+        if (quoteChar !== null) {
-          if (inString && sql[i + 1] === "'") {
+          if (ch === quoteChar) {
-            current += "''";
+            if (sql[i + 1] === quoteChar) {
-            i++;
+              current += quoteChar + quoteChar;
-            continue;
+              i++;
+              continue;
+            }
+            quoteChar = null;
           }
-          inString = !inString;
           current += ch;
           continue;
         }
-        if (ch === ';' && !inString) {
+        if (ch === "'" || ch === '"') {
+          quoteChar = ch;
+          current += ch;
+          continue;
+        }
+        if (ch === ';') {
           statements.push(current.trim());
           current = '';
           continue;
@@ -46,13 +54,13 @@
       return statements.filter(function (s) { return s !== ''; });
     }
 
-    // Быстрая клиентская проверка на DELETE — только для UX (чтобы не
+    // Быстрая клиентская проверка на DELETE/UPDATE — только для UX (чтобы не
     // делать лишний запрос к серверу). Итоговое решение всё равно
     // принимает бэкенд (requiresConfirmation), это лишь подсказка.
-    function scriptHasDelete(sql) {
+    function scriptNeedsConfirmation(sql) {
       return splitStatements(sql).some(function (part) {
         var normalized = part.replace(/^(\s*--[^\n]*\n)*\s*/, '');
-        return /^DELETE\b/i.test(normalized);
+        return /^(DELETE|UPDATE)\b/i.test(normalized);
       });
     }
 
@@ -69,15 +77,17 @@
         args: ['uid'],
         description: 'Полностью удалить локального пользователя (oc_preferences, oc_group_user, oc_ldap_user_mapping, oc_users)',
         build: function (uid) {
+          // Значение подставляется напрямую в каждый запрос (а не через
+          // SET+current_setting) — так скрипт остаётся рабочим, даже если
+          // пользователь скопирует/выполнит только часть строк по отдельности.
           var v = escapeSqlString(uid);
           return [
-            "SET vars.old_user = '" + v + "'",
+            "SELECT * FROM oc_users WHERE uid = '" + v + "'",
-            "SELECT * FROM oc_users WHERE uid = current_setting('vars.old_user')",
+            "SELECT * FROM oc_preferences WHERE userid = '" + v + "'",
-            "SELECT * FROM oc_preferences WHERE userid = current_setting('vars.old_user')",
+            "DELETE FROM oc_preferences WHERE userid = '" + v + "'",
-            "DELETE FROM oc_preferences WHERE userid = current_setting('vars.old_user')",
+            "DELETE FROM oc_group_user WHERE uid = '" + v + "'",
-            "DELETE FROM oc_group_user WHERE uid = current_setting('vars.old_user')",
+            "DELETE FROM oc_ldap_user_mapping WHERE owncloud_name = '" + v + "'",
-            "DELETE FROM oc_ldap_user_mapping WHERE owncloud_name = current_setting('vars.old_user')",
+            "DELETE FROM oc_users WHERE uid = '" + v + "'"
-            "DELETE FROM oc_users WHERE uid = current_setting('vars.old_user')"
           ].join(';\n') + ';';
         }
       },
@@ -87,11 +97,10 @@
         build: function (uid) {
           var v = escapeSqlString(uid);
           return [
-            "SET vars.old_user = '" + v + "'",
+            "SELECT * FROM oc_users WHERE uid = '" + v + "'",
-            "SELECT * FROM oc_users WHERE uid = current_setting('vars.old_user')",
+            "SELECT * FROM oc_preferences WHERE userid = '" + v + "'",
-            "SELECT * FROM oc_preferences WHERE userid = current_setting('vars.old_user')",
+            "SELECT * FROM oc_group_user WHERE uid = '" + v + "'",
-            "SELECT * FROM oc_group_user WHERE uid = current_setting('vars.old_user')",
+            "SELECT * FROM oc_ldap_user_mapping WHERE owncloud_name = '" + v + "'"
-            "SELECT * FROM oc_ldap_user_mapping WHERE owncloud_name = current_setting('vars.old_user')"
           ].join(';\n') + ';';
         }
       }
@@ -133,7 +142,9 @@
 
       function formatCell(v) {
         if (v === null || v === undefined) {
-          return '';
+          // Явная метка, а не пустая строка — иначе NULL неотличим от
+          // настоящей пустой строки '' в выводе таблицы.
+          return '[NULL]';
         }
         if (typeof v === 'object') {
           return JSON.stringify(v);
@@ -199,10 +210,15 @@
           term.echo('[[;green;]  OK (session variable set)]');
         } else if (r.type === 'delete') {
           term.echo('[[;#ff9900;]  DELETED ' + r.affected_rows + ' row(s)]');
+        } else if (r.type === 'update') {
+          term.echo('[[;#ff9900;]  UPDATED ' + r.affected_rows + ' row(s)]');
         } else {
           term.echo('[[;green;]  OK, ' + r.affected_rows + ' row(s) affected]');
         }
       });
+      if (response.rolledBack) {
+        term.echo('[[;#ff5555;]Batch failed partway through — all statements in this batch were rolled back.]');
+      }
     }
 
     function enterSqlMode(term) {
@@ -219,12 +235,18 @@
       term.echo('[[;yellow;]Switched back to OCC mode.]');
     }
 
+    // Таймаут для больших/долгих пачек (например, DELETE по большой таблице
+    // без индекса): без него зависший запрос молча оставит терминал
+    // заблокированным (term.pause()) навсегда, если сервер не ответит.
+    var SQL_REQUEST_TIMEOUT_MS = 120000;
+
     function sendSqlQuery(term, sql, confirmed) {
       term.pause();
       $.ajax({
         url: baseUrl + '/db/query',
         type: 'POST',
         contentType: 'application/json',
+        timeout: SQL_REQUEST_TIMEOUT_MS,
         data: JSON.stringify({ sql: sql, confirm: !!confirmed })
       }).done(function (response) {
         if (response && response.requiresConfirmation) {
@@ -234,14 +256,18 @@
         }
         renderSqlResponse(term, response);
         term.resume();
-      }).fail(function (xhr) {
+      }).fail(function (xhr, status) {
-        term.echo('[[;#ff5555;]Request failed: ]' + $.terminal.escape_formatting(xhr.status + ' ' + xhr.statusText));
+        if (status === 'timeout') {
+          term.echo('[[;#ff5555;]Request timed out after ' + (SQL_REQUEST_TIMEOUT_MS / 1000) + 's — the query may still be running on the server, check occ/DB logs before retrying.]');
+        } else {
+          term.echo('[[;#ff5555;]Request failed: ]' + $.terminal.escape_formatting(xhr.status + ' ' + xhr.statusText));
+        }
         term.resume();
       });
     }
 
     function askDeleteConfirmation(term, sql, message) {
-      var prompt = '[[;#ff5555;]' + (message || 'This script contains DELETE statement(s).') + ' Type "yes" to run it: ]';
+      var prompt = '[[;#ff5555;]' + (message || 'This script contains DELETE/UPDATE statement(s).') + ' Type "yes" to run it: ]';
       term.read(prompt).then(function (answer) {
         if ((answer || '').trim().toLowerCase() === 'yes') {
           sendSqlQuery(term, sql, true);
@@ -282,7 +308,7 @@
           if (!trimmed) {
             return;
           }
-          if (scriptHasDelete(command)) {
+          if (scriptNeedsConfirmation(command)) {
             askDeleteConfirmation(term, command);
           } else {
             sendSqlQuery(term, command, false);

lib/Controller/DbController.php

@@ -16,6 +16,9 @@ class DbController extends Controller
     /** Максимум строк, возвращаемых на один SELECT (защита от OOM/DoS). */
     private const MAX_ROWS = 1000;
 
+    /** Максимальный размер присланного SQL-текста в байтах (защита от OOM/DoS). */
+    private const MAX_SQL_BYTES = 1048576; // 1 MB
+
     /**
      * Конструкции, дающие доступ к файловой системе сервера или запуску
      * внешних программ через SQL. Блокируются полностью, без возможности
@@ -72,32 +75,56 @@ class DbController extends Controller
     }
 
     /**
-     * Разбивает пачку запросов по ";" с учётом одинарных кавычек, чтобы
+     * Убирает ВСЕ однострочные (-- ...) и блочные C-style комментарии
-     * точка с запятой внутри строкового литерала (в том числе с '' как
+     * из запроса, включая те, что стоят внутри выражения (например, между
-     * экранированной кавычкой) не ломала разбиение.
+     * именем функции и открывающей скобкой — иначе так можно спрятать
+     * запрещённую конструкцию от findForbiddenConstruct). Используется
+     * только для проверки на запрещённые конструкции — сам запрос на
+     * выполнение идёт без изменений.
+     */
+    private function removeAllComments($query)
+    {
+        $query = preg_replace('/--[^\n]*/', '', $query);
+        $query = preg_replace('/\/\*[\s\S]*?\*\//', '', $query);
+        return $query;
+    }
+
+    /**
+     * Разбивает пачку запросов по ";" с учётом одинарных и двойных
+     * кавычек (строки и экранированные идентификаторы Postgres), чтобы
+     * точка с запятой внутри 'строки' или "идентификатора" (в том числе
+     * с '' / "" как экранированной кавычкой) не ломала разбиение.
      */
     private function splitStatements($sql)
     {
         $statements = [];
         $current = '';
         $len = strlen($sql);
-        $inString = false;
+        $quoteChar = null;
 
         for ($i = 0; $i < $len; $i++) {
             $ch = $sql[$i];
 
-            if ($ch === "'") {
+            if ($quoteChar !== null) {
-                if ($inString && $i + 1 < $len && $sql[$i + 1] === "'") {
+                if ($ch === $quoteChar) {
-                    $current .= "''";
+                    if ($i + 1 < $len && $sql[$i + 1] === $quoteChar) {
-                    $i++;
+                        $current .= $quoteChar . $quoteChar;
-                    continue;
+                        $i++;
+                        continue;
+                    }
+                    $quoteChar = null;
                 }
-                $inString = !$inString;
                 $current .= $ch;
                 continue;
             }
 
-            if ($ch === ';' && !$inString) {
+            if ($ch === "'" || $ch === '"') {
+                $quoteChar = $ch;
+                $current .= $ch;
+                continue;
+            }
+
+            if ($ch === ';') {
                 $statements[] = trim($current);
                 $current = '';
                 continue;
@@ -118,11 +145,14 @@ class DbController extends Controller
     /**
      * Возвращает описание найденной запрещённой конструкции (доступ к ФС,
      * запуск программ) или null, если запрос безопасен в этом плане.
+     * Проверяется версия запроса без комментариев — иначе конструкцию
+     * можно спрятать, вставив комментарий между именем функции и "(".
      */
     private function findForbiddenConstruct($query)
     {
+        $clean = $this->removeAllComments($query);
         foreach (self::FORBIDDEN_PATTERNS as $pattern => $label) {
-            if (preg_match($pattern, $query)) {
+            if (preg_match($pattern, $clean)) {
                 return $label;
             }
         }
@@ -151,6 +181,13 @@ class DbController extends Controller
             return new JSONResponse(['success' => false, 'error' => 'Empty query']);
         }
 
+        if (strlen($sql) > self::MAX_SQL_BYTES) {
+            return new JSONResponse([
+                'success' => false,
+                'error' => 'Query too large (max ' . self::MAX_SQL_BYTES . ' bytes)'
+            ], 413);
+        }
+
         // Аудит: логируем сам факт попытки выполнения ДО всех проверок,
         // чтобы в логе остались и заблокированные/отклонённые запросы,
         // а не только успешно выполненные.
@@ -186,31 +223,53 @@ class DbController extends Controller
             }
         }
 
-        // DELETE необратим, поэтому требуем явное подтверждение с клиента
+        // DELETE и UPDATE необратимы (или трудно обратимы), поэтому
-        // (confirm=true), прежде чем выполнять хоть один запрос из пачки.
+        // требуем явное подтверждение с клиента (confirm=true), прежде
+        // чем выполнять хоть один запрос из пачки.
         $deleteCount = 0;
+        $updateCount = 0;
         foreach ($queries as $query) {
-            if (stripos($this->stripLeadingComments($query), 'DELETE') === 0) {
+            $normalized = $this->stripLeadingComments($query);
+            if (stripos($normalized, 'DELETE') === 0) {
                 $deleteCount++;
+            } elseif (stripos($normalized, 'UPDATE') === 0) {
+                $updateCount++;
             }
         }
 
-        if ($deleteCount > 0 && !$confirmed) {
+        if (($deleteCount > 0 || $updateCount > 0) && !$confirmed) {
+            $parts = [];
+            if ($deleteCount > 0) {
+                $parts[] = "{$deleteCount} DELETE";
+            }
+            if ($updateCount > 0) {
+                $parts[] = "{$updateCount} UPDATE";
+            }
             return new JSONResponse([
                 'success' => false,
                 'requiresConfirmation' => true,
                 'deleteCount' => $deleteCount,
-                'error' => "Batch contains {$deleteCount} DELETE statement(s) and was not executed. Resend with confirm=true to proceed."
+                'updateCount' => $updateCount,
+                'error' => 'Batch contains ' . implode(' and ', $parts) . ' statement(s) and was not executed. Resend with confirm=true to proceed.'
             ]);
         }
 
+        // Пачка выполняется в одной транзакции: если один из запросов
+        // упадёт (например, DELETE на середине серии из-за FK), все уже
+        // выполненные в этой же пачке изменения откатываются, а не
+        // остаются частично применёнными. SET (без LOCAL) не транзакционен
+        // в PostgreSQL, поэтому откат не затрагивает current_setting().
         $results = [];
+        $rolledBack = false;
+
+        $this->db->beginTransaction();
 
         foreach ($queries as $query) {
             $normalized = $this->stripLeadingComments($query);
             $isSelect = stripos($normalized, 'SELECT') === 0;
             $isSet = stripos($normalized, 'SET ') === 0;
             $isDelete = stripos($normalized, 'DELETE') === 0;
+            $isUpdate = stripos($normalized, 'UPDATE') === 0;
 
             try {
                 $stmt = $this->db->prepare($query);
@@ -238,7 +297,15 @@ class DbController extends Controller
                     ];
                 } else {
                     $affected = $stmt->rowCount();
-                    $type = $isSet ? 'set' : ($isDelete ? 'delete' : 'write');
+                    if ($isSet) {
+                        $type = 'set';
+                    } elseif ($isDelete) {
+                        $type = 'delete';
+                    } elseif ($isUpdate) {
+                        $type = 'update';
+                    } else {
+                        $type = 'write';
+                    }
                     $results[] = [
                         'query' => $query,
                         'type' => $type,
@@ -251,13 +318,23 @@ class DbController extends Controller
                     'type' => 'error',
                     'error' => $e->getMessage()
                 ];
-                // Останавливаемся на первой ошибке: не продолжаем выполнять
+                // Откатываем всю пачку и останавливаемся: не продолжаем
-                // оставшиеся запросы пачки (например, серию DELETE),
+                // выполнять оставшиеся запросы (например, серию DELETE),
                 // если один из предыдущих шагов не выполнился.
+                $this->db->rollBack();
+                $rolledBack = true;
                 break;
             }
         }
 
-        return new JSONResponse(['success' => true, 'results' => $results]);
+        if (!$rolledBack) {
+            $this->db->commit();
+        }
+
+        return new JSONResponse([
+            'success' => true,
+            'rolledBack' => $rolledBack,
+            'results' => $results
+        ]);
     }
 }