A Link Could Change Data

Security: a link could change data

Fixed in 3.15.1 · Reported privately by a security researcher, who asked not to be named · Present in every release up to and including 3.15.0 · Modules: Tickets, System, Assets, Software, Knowledge, CMDB, Messaging, My Account

---

The short version

FreeITSM refuses a request from another website that tries to change something — "save this setting", "delete that" — unless it carries a secret only a FreeITSM page could know. But it only asked for that secret on POST, PUT, PATCH and DELETE, on the reasonable rule that a GET — the kind of request a plain link makes — never changes anything.

About twenty endpoints broke that rule. So a link, on any web page or in any email, could make whoever clicked it — while signed in to FreeITSM — do something they never chose to:

• empty their company's ticket trash, permanently, with every attachment, note and history row;
• as an administrator, switch module permissions from "least" to "most", widening what every analyst can open;
• wipe the SLA settings and turn off the warning and breach notifications;
• blank the branding, clear the public base URL (which breaks the ticket links in outgoing email), reset asset label and tag numbering settings, switch Apache Tika off;
• as an administrator, run Database Verification, or delete a ticket through a debug tool.

Who could do it: anybody who could get a signed-in analyst to click a link, or load a page with an image pointed at one. They needed no account. They could not see anything — the response goes to the victim's browser, not to them — so this was about damage, not theft. Each action needed a victim with the right permission: the trash needed the Tickets module, the SLA settings needed the SLA permission, the module permission switch needed an administrator.

Upgrading is the fix. Nothing else needs doing.

What was wrong

Every request with the session cookie runs through csrfEnforce() in includes/csrf.php, which checks where it came from and that it carries the session's token. It begins:

if (PHP_SAPI === 'cli' || !csrfIsStateChanging()) return;

and csrfIsStateChanging() is true for anything except GET, HEAD and OPTIONS. So a GET is never checked — on purpose, and the file said so:

NOT CHECKED: GET/HEAD/OPTIONS (they must not change anything …)

The session cookie is SameSite=Lax, and Lax cookies are sent on a top-level GET from another site — that is what lets a link in an email open a ticket without signing in again. Put the two together and any endpoint that writes on a GET is reachable from a link, with the victim's session, past both checks.

api/tickets/empty_trash.php is the plainest case. It takes no input at all — it empties the trash of the active company — and it never looked at the request method. The app's own Empty trash button sends a POST. Nothing stopped a GET.

The quieter half: endpoints that only read a JSON body

The more interesting finding came from the sweep, not the report. Most of FreeITSM's save endpoints read their input from a JSON body:

$input = json_decode(file_get_contents('php://input'), true);
$mode = ($input['mode'] ?? '') === 'least' ? 'least' : 'most';
// ... save $mode

That looks safe from a link — a link cannot send a body. But it doesn't need to. With no body, $input is null, every field falls back to its default, and if the defaults pass validation, they are saved. save_module_permission_mode.php saves most. save_sla_global_settings.php saves seven empty SLA settings. save_branding.php saves six empty header and footer slots. None of them read the query string at all, which is exactly why a scan for "endpoints that read $_GET and write" would never have found them.

It was twenty-two places, not four

The report named empty_trash.php, debug tool D002, Database Verification and the messaging channel test, and said plainly that its scan was short and not complete. Every one was real.

The sweep looked at all 339 files under api/ that contain write SQL, in three groups: those that read the query string, those that read nothing, and the 260 that read only a body. The body group was read file by file, with one question: with an empty body, does this still reach a write? Twelve of the 260 did. Then the 51 files that already checked the method were read again, because checking the method is not the same as checking it in the right place — and one of them wasn't.

Endpoint	What a link did	The victim needed
api/tickets/empty_trash.php	permanently deleted the company's trashed tickets	Tickets
api/system/debug-tools/D002_delete_ticket.php	deleted the ticket named in ?ref=	administrator
api/system/db_verify.php	ran Database Verification	administrator
api/messaging/test_channel.php	?mode=simulate created and deleted a test ticket and requester	messaging permission
api/system/save_module_permission_mode.php	switched "least" to "most"	administrator
api/system/save_branding.php	blanked the header and footer text	administrator
api/tickets/save_sla_global_settings.php	wiped seven SLA settings, notifications off	SLA permission
api/tickets/save_public_base_url.php	cleared the public base URL — [ticket_url] links in email break	email templates permission
api/messaging/save_base_url.php	cleared the messaging webhook base URL	messaging permission
api/assets/save_asset_label_settings.php	reset the label layout	label permission
api/assets/save_asset_tag_settings.php	turned tag numbering off, reset the format	tag permission
api/cmdb/save_ai_settings.php	cleared the CMDB AI instructions	CMDB AI permission
api/knowledge/permission_model.php	?action=set forced the folder permission model to "containers"	Knowledge manage
api/system/tika_settings.php	?action=save switched Apache Tika off	administrator
api/myaccount/save_profile.php	wiped the victim's job title, department and phone numbers	any analyst
api/myaccount/toggle_trust_device.php	flipped trusted devices, clearing them	any analyst
api/software/generate_apikey.php	created an unwanted live inventory API key	API keys permission
api/assets/get_vcenter.php	ran a vCenter sync	Assets
api/tickets/check_mailbox_email.php	checked a mailbox early	Tickets
api/tickets/set_base_url_warning_dismissed.php	brought back a dismissed warning	email templates permission
api/system/debug-tools/D001_demo_core_import.php	imported the demo data - which includes working analyst accounts with a password published in the repository, so on an install that had never loaded it, one click gave an outsider a way in. The worst of the twenty-two	administrator
api/system/debug-tools/D007_search_corpus.php	wrote and removed a probe row (harmless)	administrator

knowledge/permission_model.php is the one worth studying. It did look at the method — and used it to decide where to read the action from: the query string on a GET, the body otherwise. So ?action=set on a GET went straight to the save, with an empty input whose default was "containers". A method check that routes rather than refuses is not a protection.

The fix

One rule, stated where it can be enforced: an endpoint that writes refuses a GET. includes/csrf.php gains:

function csrfRequireWriteMethod(): void
{
    if (PHP_SAPI === 'cli' || csrfIsStateChanging()) return;
    http_response_code(405);
    header('Allow: POST');
    header('Content-Type: application/json');
    echo json_encode(['success' => false, 'error' => 'This request must be sent as a POST.']);
    exit;
}

and every endpoint under api/ that writes calls it before its first write — 276 files, not just the twenty-two. The narrow fix would have closed today's holes and left the rule exactly as unenforced as it was; the next save endpoint whose defaults happen to validate would reopen it. Where an endpoint reads on GET and writes on POST (Tika, checklists, the Knowledge permission model), the call goes in the write branch, so the reads keep working.

Three screens sent a GET to an endpoint that writes and now send a POST: System → Database Verification, the setup wizard's schema step, and the Servers page's vCenter sync. The debug tool registry marks D001, D002 and D007 as POST, which the tool page already supported.

A handful of GETs keep writing, deliberately, and are listed with their reasons in the guard test: a download log, two article view counters, marking an email read, and the single sign-on callback (which checks its own state against the session).

Why not fix it centrally?

The tempting fix is one line in csrfEnforce(): check GETs too. It cannot work — a link to an attachment, a ticket or a download is a cross-site GET with the cookie, and must keep working. The browser's Sec-Fetch-Site header cannot tell "a link that opens a ticket" from "a link that empties the trash" either: both are top-level navigations from another site. Only the endpoint knows whether it writes, so the endpoint has to say so — and a test has to make sure it does.

Making it stay fixed

The developer guide had the rule all along: "Change data on POST …, never on GET. A GET is not checked." It also had, three sections earlier, the reason that rule was never going to hold:

a protection that every future endpoint has to remember is not a protection — the first one somebody forgets is the hole.

So the rule is now a test. tests/get-must-not-write.php reads every file under api/, strips comments with PHP's own tokenizer, and fails for any file that contains write SQL and none of csrfRequireWriteMethod(), csrfRequireToken() or a REQUEST_METHOD check. It runs on every push, in the Web exposure workflow.

📁 Files changed

	File	Change
🟦	includes/csrf.php	csrfRequireWriteMethod(); the header now says the GET rule is enforced, and where
🟩	276 files under api/	one call each, before the first write (three lines, a comment and the call)
🟨	api/system/tika_settings.php, api/tickets/ticket_checklists.php, api/knowledge/permission_model.php	the call inside the write branches only
🟧	system/db-verify/index.php, setup/index.php, asset-management/servers/index.php	send POST
🟧	system/debug-tools/includes/tools.php	D001, D002, D007 run as POST
🟪	tests/get-must-not-write.php, .github/workflows/web-exposure.yml	the guard, on every push
🟪	tests/security-report-2026-10.php	the live checks

Tested

• The guard bites. Against the code as it was in 3.15.0 it reports 277 writers with no check; against 3.15.1, none. It tests itself first against fixtures, including a check that appears only in a comment (must fail) and SQL that appears only in a comment (must not count).
• Live, through the real web server (tests/security-report-2026-10.php): a GET to fifteen of the endpoints above is refused. 🔑 These run signed out on purpose — if a fix were missing, a signed-out request still could not write, so the test can never empty a real trash. One signed-in check runs as an administrator, against the module permission mode, which it snapshots first: the GET is refused with 405 and the value is unchanged; the same save as a POST with the token still works (the positive control); and a POST without the token is still refused (the CSRF check is still there).
• A read on GET still works (Tika settings, action=get), and debug tool D007 runs as the POST the tool page now sends.
• The rest of the suite — 32 test files, including tests/csrf.php — passes unchanged.

Not run live: Database Verification and D002 as a POST. One migrates the schema and the other deletes a ticket, and the change to each was the method alone.

Do I need to do anything?

Upgrade. Nothing to configure. If you have your own scripts calling FreeITSM's internal endpoints (not the REST API under /api/v1/, which is unaffected) and any of them use GET to save something, they will now get 405 and must use POST.

For developers

• An endpoint that changes anything calls csrfRequireWriteMethod() before it writes. That is the whole rule. See CSRF protection — Developer Guide §8.1.
• "It only reads a JSON body" is not an exemption. An empty body is a valid input; ask what your defaults save.
• A method check must refuse, not route. If you read the action from the query string on a GET, put the call in each write branch.
• A GET that genuinely must write — a view counter — goes on the allow-list in tests/get-must-not-write.php, with its reason.

Related: CSRF protection — Developer Guide · Access outlived its removal · Smaller gaps from the same reports · Bugs resolved