diff --git a/agent/post/asset.php b/agent/post/asset.php index e5db2ec0..b82a07f4 100644 --- a/agent/post/asset.php +++ b/agent/post/asset.php @@ -18,6 +18,19 @@ if (isset($_POST['add_asset'])) { enforceClientAccess(); + // Only the two credential fields this handler writes - name/description/uri here + // belong to the asset, not the credential, and have their own column widths. + // Checked before the asset is created, so an overlong credential can't leave a + // half-built asset behind. Form maxlength doesn't reach a hand-rolled POST. + if ($credential_field_too_long = checkCredentialLengths([ + 'username' => $_POST['username'] ?? null, + 'password' => $_POST['password'] ?? null, + ])) { + flashAlert("Credential $credential_field_too_long is too long to store", 'error'); + redirect(); + exit; + } + $alert_extended = ""; mysqli_query($mysqli,"INSERT INTO assets SET asset_name = '$name', asset_description = '$description', asset_type = '$type', asset_make = '$make', asset_model = '$model', asset_serial = '$serial', asset_os = '$os', asset_uri = '$uri', asset_uri_2 = '$uri_2', asset_uri_client = '$uri_client', asset_location_id = $location, asset_vendor_id = $vendor, asset_contact_id = $contact, asset_status = '$status', asset_purchase_reference = '$purchase_reference', asset_purchase_date = $purchase_date, asset_warranty_expire = $warranty_expire, asset_install_date = $install_date, asset_physical_location = '$physical_location', asset_notes = '$notes', asset_favorite = $favorite, asset_client_id = $client_id"); diff --git a/agent/post/credential.php b/agent/post/credential.php index 355a1b0c..23d3a9c6 100644 --- a/agent/post/credential.php +++ b/agent/post/credential.php @@ -563,8 +563,24 @@ if (isset($_POST["import_credentials_csv"])) { fgetcsv($file, 1000, ","); // Skip first line $row_count = 0; $duplicate_count = 0; + $too_long_count = 0; while(($column = fgetcsv($file, 1000, ",")) !== false){ $duplicate_detect = 0; + + // Nothing client-side guards an uploaded file, and an overlong value is a hard + // MySQL error - skip the row and report it rather than losing the whole import + if (checkCredentialLengths([ + 'name' => $column[0] ?? null, + 'description' => $column[1] ?? null, + 'username' => $column[2] ?? null, + 'password' => $column[3] ?? null, + 'otp_secret' => $column[4] ?? null, + 'uri' => $column[5] ?? null, + ])) { + $too_long_count = $too_long_count + 1; + continue; + } + // Name if (isset($column[0])) { $name = escapeSql($column[0]); @@ -589,7 +605,7 @@ if (isset($_POST["import_credentials_csv"])) { $totp = escapeSql($column[4]); } // URL - if (isset($column[4])) { + if (isset($column[5])) { $uri = escapeSql($column[5]); } @@ -604,9 +620,9 @@ if (isset($_POST["import_credentials_csv"])) { } fclose($file); - logAudit("Credential", "Import", "$session_name imported $row_count credential(s) via CSV file. $duplicate_count duplicate(s) found and not imported", $client_id); + logAudit("Credential", "Import", "$session_name imported $row_count credential(s) via CSV file. $duplicate_count duplicate(s) found and not imported, $too_long_count row(s) skipped for over-length fields", $client_id); - flashAlert("$row_count credential(s) imported, $duplicate_count duplicate(s) detected and not imported", 'warning'); + flashAlert("$row_count credential(s) imported, $duplicate_count duplicate(s) detected and not imported, $too_long_count row(s) skipped for over-length fields", 'warning'); redirect(); } diff --git a/agent/post/credential_model.php b/agent/post/credential_model.php index 4d839ac2..806814a4 100644 --- a/agent/post/credential_model.php +++ b/agent/post/credential_model.php @@ -2,6 +2,13 @@ // Model of reusable variables for client credentials - not to be confused with the ITFLow login process defined('FROM_POST_HANDLER') || die("Direct file access is not allowed"); +// The form maxlength is client-side only - a hand-rolled POST gets here without it +if ($credential_field_too_long = checkCredentialLengths($_POST)) { + flashAlert("Credential $credential_field_too_long is too long to store", 'error'); + redirect(); + exit; +} + $name = escapeSql($_POST['name']); $description = escapeSql($_POST['description']); $uri = escapeSql($_POST['uri']); diff --git a/api/v1/credentials/credential_model.php b/api/v1/credentials/credential_model.php index 8d37b09b..24562f63 100644 --- a/api/v1/credentials/credential_model.php +++ b/api/v1/credentials/credential_model.php @@ -3,6 +3,29 @@ // Variable assignment from POST (or: blank/from DB is updating) +/* + * There is no form behind the API, so nothing has capped these before they arrive. + * An overlong value is a hard MySQL error, not a truncation, so it would surface as a + * generic "insert query failed" - say what actually went wrong instead. + * Only the fields present are checked, which keeps partial updates working. + */ +$credential_field_too_long = checkCredentialLengths([ + 'name' => $_POST['credential_name'] ?? null, + 'description' => $_POST['credential_description'] ?? null, + 'uri' => $_POST['credential_uri'] ?? null, + 'uri_2' => $_POST['credential_uri_2'] ?? null, + 'username' => $_POST['credential_username'] ?? null, + 'password' => $_POST['credential_password'] ?? null, + 'otp_secret' => $_POST['credential_otp_secret'] ?? null, +]); + +if ($credential_field_too_long) { + $return_arr['success'] = "False"; + $return_arr['message'] = "credential_$credential_field_too_long is too long to store."; + echo json_encode($return_arr); + exit(); +} + $api_key_decrypt_password = ''; if (isset($_POST['api_key_decrypt_password'])) { $api_key_decrypt_password = $_POST['api_key_decrypt_password']; // No sanitization diff --git a/functions/security.php b/functions/security.php index 24c9632f..ef59dd50 100644 --- a/functions/security.php +++ b/functions/security.php @@ -175,6 +175,60 @@ function apiEncryptCredentialEntry(#[\SensitiveParameter]$credential_cleartext, return $iv . $ciphertext; } +/* + * Longest cleartext a credential username or password may be. + * Both encrypt functions above return a 16-char IV followed by base64 AES-128-CBC + * ciphertext, which expands about 1.37x, so 350 is the most that still fits the + * varchar(500) columns. Keep in step with the maxlength on the credential/asset forms. + */ +define('CREDENTIAL_ENTRY_MAX_LENGTH', 350); + +/* + * Checks a credential's cleartext fields against what the columns can actually store. + * Form maxlength is client-side only, so the CSV import, the API and any hand-rolled POST + * reach the INSERT with nothing stopping an overlong value - and MySQL rejects it outright + * rather than truncating, taking the request down with it. + * + * Returns the name of the first field that is too long, or an empty string when they all + * fit. Only keys actually present are checked, so partial updates are fine. + */ +function checkCredentialLengths(array $fields) { + + // Encrypted before storage - ciphertext size follows the BYTE length of the cleartext. + $byte_limits = [ + 'username' => CREDENTIAL_ENTRY_MAX_LENGTH, + 'password' => CREDENTIAL_ENTRY_MAX_LENGTH, + ]; + + // Stored as given - MySQL measures varchar in CHARACTERS, not bytes. + $char_limits = [ + 'name' => 200, + 'description' => 500, + 'uri' => 500, + 'uri_2' => 500, + 'otp_secret' => 200, + ]; + + foreach ($byte_limits as $field => $limit) { + if (isset($fields[$field]) && strlen($fields[$field]) > $limit) { + return $field; + } + } + + foreach ($char_limits as $field => $limit) { + if (!isset($fields[$field]) || strlen($fields[$field]) <= $limit) { + continue; // byte length caps character count, so this already fits + } + // Only worth counting characters once the cheap check fails. preg keeps this + // free of an mbstring dependency, which nothing else in the tree relies on. + if (preg_match_all('/./us', $fields[$field]) > $limit) { + return $field; + } + } + + return ''; +} + // Cross-Site Request Forgery check for sensitive functions // Validates the CSRF token provided matches the one in the users session function validateCSRFToken(?string $token = null) {