AS1024/itflow
1
0
Fork 0
mirror of https://github.com/itflow-org/itflow synced 2026-09-02 04:45:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix approval Syntax

Browse Source
This commit is contained in:
johnnyq
2026-08-31 17:52:44 -04:00
parent 7442e0c2c8
commit bcdfab996c
6 changed files with 35 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
CONTRIBUTING.md
View File

@@ -146,7 +146,7 @@ ITFlow does not use prepared statements or an ORM; queries are built as strings.
- **Integers** (IDs, flags, counts): `intval($_POST['ticket_id'])`. Interpolate unquoted.
- **Strings**: `escapeSql($_POST['subject'])`. This normalizes encoding to UTF-8, then runs `strip_tags()`, `trim()`, and `mysqli_real_escape_string()`. Because it relies on SQL escaping, the value **must be placed inside quotes in the query** (`'$subject'`). An escaped string interpolated without quotes is still injectable.
- **Values read back from the database** get the same treatment before reuse in another query (you will see `escapeSql($row['ticket_prefix'])` throughout — this is why).
- **`logAudit()`, `appNotify()`, `logHistory()` and `logTicketHistory()` are queries too.** They interpolate their `$description` / `$details` / `$status` arguments straight into an `INSERT` — the SQL is just hidden inside the helper. A DB-read value passed into one of them (`logAudit("Asset", "Delete", "$asset_name ...", ...)`) must be `escapeSql`'d first, exactly as if you had written the `INSERT` by hand. This is easy to miss precisely because the call doesn't *look* like a query.
- **`logAudit()`, `appNotify()`, `logHistory()` and `logTicketHistory()` are queries too.** They interpolate their `$description` / `$details` / `$status` arguments straight into an `INSERT` — the SQL is just hidden inside the helper. A DB-read value passed into one of them (`logAudit("Asset", "Delete", "$asset_name ...", ...)`) must be `escapeSql`'d first, exactly as if you had written the `INSERT` by hand. This is easy to miss precisely because the call doesn't *look* like a query. Note that `escapeHtml()` is **not** a substitute here: it encodes `'` and `"` so it happens to block a quote-breakout, but it leaves backslashes untouched, so a value ending in `\` still escapes the closing quote. All four sinks now trim an odd trailing backslash as a backstop, but the value still owes `escapeSql` — the guard is defence-in-depth, not the fix.
If you write a query and even one variable in it skipped these, that is a SQL injection. This is the single most common review rejection.
**Fetch helpers return raw values — you escape them.** `getFieldById()` and `getTicketStatusName()` hand back exactly what is in the column. Escaping is the call site's job, the same as any other row you read:

3
admin/post/saved_payment_method.php
View File

@@ -61,7 +61,8 @@ if (isset($_GET['delete_saved_payment'])) {
// SQL Cascade delete will Remove All Associated Auto Payment Methods on recurring invoices in the recurring payments table.
logAudit("Payment Provider", "Update", "$session_name deleted saved payment method $saved_payment_description (PM: $payment_method)", $client_id);
$payment_method_esc = escapeSql($payment_method);
logAudit("Payment Provider", "Update", "$session_name deleted saved payment method $saved_payment_description (PM: $payment_method_esc)", $client_id);
flashAlert("Payment method <strong>$saved_payment_description</strong> removed", 'error');

2
agent/post/task.php
View File

@@ -351,7 +351,7 @@ if (isset($_GET['approve_ticket_task'])) {
$approval_row = mysqli_fetch_assoc(mysqli_query($mysqli, "SELECT approval_created_by, approval_required_user_id, approval_scope, approval_type, task_name,
task_ticket_id FROM task_approvals LEFT JOIN tasks on task_id = approval_task_id WHERE approval_id = $approval_id AND approval_task_id = $task_id AND approval_scope = 'internal'"));
$task_name = escapeHtml($approval_row['task_name']);
$task_name = escapeSql($approval_row['task_name']);
$scope = escapeHtml($approval_row['approval_scope']);
$type = escapeHtml($approval_row['approval_type']);
$required_user = intval($approval_row['approval_required_user_id']);

2
client/post.php
View File

@@ -277,7 +277,7 @@ if (isset($_GET['approve_ticket_task'])) {
$approval_row = mysqli_fetch_assoc(mysqli_query($mysqli, "SELECT approval_created_by, approval_required_user_id, approval_scope, approval_type, task_name,
task_ticket_id FROM task_approvals LEFT JOIN tasks on task_id = approval_task_id WHERE approval_id = $approval_id AND approval_task_id = $task_id AND approval_url_key = '$url_key' AND approval_status = 'pending' AND approval_scope = 'client'"));
$task_name = escapeHtml($approval_row['task_name']);
$task_name = escapeSql($approval_row['task_name']);
$scope = escapeHtml($approval_row['approval_scope']);
$type = escapeHtml($approval_row['approval_type']);
$required_user = intval($approval_row['approval_required_user_id']);

29
functions/logging.php
View File

@@ -22,10 +22,26 @@ function appNotify($type, $details, $action = null, $client_id = 0, $entity_id =
$action = "NULL"; // Without quotes for SQL NULL
}
$client_id = intval($client_id);
$entity_id = intval($entity_id);
$type = substr($type, 0, 200);
$details = substr($details, 0, 1000);
$action = substr($action, 0, 250);
// Callers pass values that are already SQL-safe, but cutting at a fixed
// length (or an escaper that leaves backslashes, e.g. escapeHtml) can leave
// an odd trailing backslash that would escape this query's closing quote
if ((strlen($type) - strlen(rtrim($type, '\\'))) % 2 === 1) {
$type = substr($type, 0, -1);
}
if ((strlen($details) - strlen(rtrim($details, '\\'))) % 2 === 1) {
$details = substr($details, 0, -1);
}
if ((strlen($action) - strlen(rtrim($action, '\\'))) % 2 === 1) {
$action = substr($action, 0, -1);
}
$sql = mysqli_query($mysqli, "SELECT user_id FROM users
WHERE user_type = 1 AND user_status = 1 AND user_archived_at IS NULL
");
@@ -52,6 +68,19 @@ function logAudit($type, $action, $description, $client_id = 0, $entity_id = 0)
$action = substr($action, 0, 255);
$description = substr($description, 0, 1000);
// Callers pass values that are already SQL-safe, but cutting at a fixed
// length (or an escaper that leaves backslashes, e.g. escapeHtml) can leave
// an odd trailing backslash that would escape this query's closing quote
if ((strlen($type) - strlen(rtrim($type, '\\'))) % 2 === 1) {
$type = substr($type, 0, -1);
}
if ((strlen($action) - strlen(rtrim($action, '\\'))) % 2 === 1) {
$action = substr($action, 0, -1);
}
if ((strlen($description) - strlen(rtrim($description, '\\'))) % 2 === 1) {
$description = substr($description, 0, -1);
}
mysqli_query($mysqli, "INSERT INTO logs SET log_type = '$type', log_action = '$action', log_description = '$description', log_ip = '$session_ip', log_user_agent = '$session_user_agent', log_client_id = $client_id, log_user_id = $session_user_id, log_entity_id = $entity_id");
}

2
guest/guest_post.php
View File

@@ -260,7 +260,7 @@ if (isset($_GET['approve_ticket_task'])) {
$approval_row = mysqli_fetch_assoc(mysqli_query($mysqli, "SELECT approval_created_by, approval_required_user_id, approval_scope, approval_type, task_name,
task_ticket_id FROM task_approvals LEFT JOIN tasks on task_id = approval_task_id WHERE approval_id = $approval_id AND approval_task_id = $task_id AND approval_url_key = '$url_key' AND approval_status = 'pending'"));
$task_name = escapeHtml($approval_row['task_name']);
$task_name = escapeSql($approval_row['task_name']);
$scope = escapeHtml($approval_row['approval_scope']);
$type = escapeHtml($approval_row['approval_type']);
$required_user = intval($approval_row['approval_required_user_id']);