Skip to content

Knowledge Writes Ignored Permissions

Ed Mozley edited this page Oct 24, 2026 · 1 revision

Security: Knowledge writes ignored folder and article permissions

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: Knowledge, REST API


The short version

Knowledge lets you restrict a folder, or a single article, to named analysts and teams. Somebody left off the list does not see it in the tree, in search, or by opening it.

But the actions that change an article only checked the article's company β€” not its folder or its own access list. So an analyst who could not read an article could still, knowing its id:

  • edit it β€” title and body;
  • move it into another folder, or change its audience (one at a time, or with the bulk audience change);
  • archive it, restore it from the recycle bin, or purge it for good.

And anybody could file a new article into a folder they could not read.

Through the REST API, the same gap had a second half: creating, updating or restoring an article returned the whole article, body included, without asking whether the key was allowed to read it.

Who could do it: an analyst with the Knowledge module, or a REST API key that can write articles. They needed the article's id β€” ids are sequential, so that is a matter of counting. They could not see a restricted article's text through the Knowledge page; they could damage or delete it, and through the REST API they could read it in the response.

It only matters if you use access rules. An install with no restricted folders, no restricted articles and no access lists has nothing to bypass β€” every analyst may read every article in their company.

Upgrading is the fix. Nothing else needs doing.

What was wrong

The company check, and nothing else

Every write in includes/services/knowledge.php β€” saveArticle(), archiveArticle(), restoreArticle(), purgeArticle() β€” begins with the same guard, assertCanAccessArticle(). It was:

private static function assertCanAccessArticle(PDO $conn, ActorContext $ctx, int $articleId): void
{
    $scope = $ctx->companyScope;
    if (!is_array($scope)) return;                     // all companies

    $stmt = $conn->prepare("SELECT tenant_id FROM knowledge_articles WHERE id = ?");
    $stmt->execute([$articleId]);
    $row = $stmt->fetch(PDO::FETCH_ASSOC);
    if (!$row) return;                                  // loadArticleRow raises the 404
    if ($row['tenant_id'] === null) return;             // shared with everyone

    if (!in_array((int)$row['tenant_id'], array_map('intval', $scope), true)) {
        throw new ServiceError('forbidden', 'forbidden', 'That article belongs to a company you do not have access to.');
    }
}

Its name says "can access article". What it checks is "same company". Those were the same question before folder permissions existed, and the guard was never revisited when they arrived. Note the first early return: an analyst who can reach all companies β€” the commonest kind on a single-company install β€” was waved through before anything was looked at.

The bulk audience change (api/knowledge/knowledge_bulk_audience.php) is a loop over saveArticle(), so it inherited the same gap.

The folder check: does it exist?

When a save named a folder, resolveFolderId() checked only this:

$st = $conn->prepare("SELECT 1 FROM knowledge_folders WHERE id = ?");
$st->execute([$id]);
if (!$st->fetchColumn()) {
    throw new ServiceError('validation', 'invalid_field', "'folder_id' is not a known folder.");
}

β€” whether the folder exists, not whether you may read it.

The check existed one file away

The folder tree's drag-and-drop, api/knowledge/folders.php, moves an article between folders too, and it was right all along. It checks both ends: the folder you are dropping into, and the article you are dragging.

requireFolderAccess($conn, $analystId, $folder);

$viewer = KnowledgeViewer::forAnalyst($conn, $analystId);
if (!knowledgeCanRead($conn, $viewer, $articleId, ['lifecycle' => 'any'])) {
    echo json_encode(['success' => false, 'error' => 'Article not found']);
    return;
}

So "move this article" was checked when you dragged it, and not checked when you changed its folder in the editor. Same action, two doors, one locked.

The REST API: a check that was there, and not passed

api/v1/resources/knowledge.php loads an article through apiLoadArticle(), which already runs the key's read check β€” when it is given the key:

if ($apiKey !== null
    && !knowledgeCanRead($conn, KnowledgeViewer::forApiKey($conn, $apiKey), $articleId, ['lifecycle' => 'any'])) {
    apiError(404, 'not_found', 'Article not found.');
}

Create, update and restore called it without one:

apiRespond(apiSerializeArticle($conn, apiLoadArticle($conn, $res['id']), true), 201);

so the check was skipped and the full article came back.

The fix

Writes ask the same question reads do

assertCanAccessArticle() keeps the company check, and adds the read rule:

$scope = $ctx->companyScope;
if (is_array($scope) && $row['tenant_id'] !== null   // null = shared with everyone
    && !in_array((int)$row['tenant_id'], array_map('intval', $scope), true)) {
    throw new ServiceError('forbidden', 'forbidden', 'That article belongs to a company you do not have access to.');
}

$viewer = self::viewerFor($conn, $ctx);
if ($viewer && !knowledgeCanRead($conn, $viewer, $articleId, ['lifecycle' => 'any'])) {
    throw new ServiceError('not_found', 'not_found', 'Article not found.');
}

The early "all companies" return is gone, so an all-companies analyst now reaches the read check too. knowledgeCanRead() is the function that decides what the Knowledge page shows, so a write is now allowed exactly when a read would be. lifecycle => any is there because restore and purge act on archived articles, which the default read would not include.

πŸ”‘ The refusal says "Article not found." β€” the same words as an id that does not exist. A different message would tell a caller which ids are real articles they are not allowed to see.

knowledgeCanRead() needs to know who is asking, as a KnowledgeViewer. The service is handed an ActorContext, so it builds one:

private static function viewerFor(PDO $conn, ActorContext $ctx): ?KnowledgeViewer
{
    if ($ctx->actorId <= 0) return null;
    return $ctx->source === 'api'
        ? KnowledgeViewer::forApiKey($conn, ['analyst_id' => $ctx->actorId, 'company_scope' => $ctx->companyScope])
        : KnowledgeViewer::forAnalyst($conn, $ctx->actorId);
}

An analyst is judged as the analyst; an API key by its own company scope; the system actor (internal callers, with no analyst) is not restricted, as before.

Holders of the Knowledge manage permission keep the override they already had: knowledgeCanRead() lets them in and writes an admin_override row to the article's history, which is what keeps a restricted folder recoverable when its only member leaves.

A folder you cannot read is not a folder

resolveFolderId() now takes the actor, and treats an unreadable folder exactly like a missing one:

$viewer = self::viewerFor($conn, $ctx);
if (!$st->fetchColumn() || ($viewer && !knowledgeFolderIsReadable($conn, $viewer, $id))) {
    throw new ServiceError('validation', 'invalid_field', "'folder_id' is not a known folder.");
}

knowledgeFolderIsReadable() used to be a private helper inside api/knowledge/folders.php. It has moved, unchanged, into includes/knowledge/visibility.php, beside the rest of the read rules, and both the folder tree and the article service now call the same copy. Writing a second, similar check into the service would have fixed today's hole and set up the next one: two rules for "may this person reach this folder" drift, and the copy that falls behind is the one nobody is looking at.

The REST API passes the key

Create, update and restore now hand apiLoadArticle() the key:

apiRespond(apiSerializeArticle($conn, apiLoadArticle($conn, $res['id'], $apiKey), true), 201);

πŸ“ Files changed

File Change
🟦 includes/services/knowledge.php assertCanAccessArticle() also requires knowledgeCanRead(); new viewerFor(); resolveFolderId() checks the folder is readable
🟩 includes/knowledge/visibility.php knowledgeFolderIsReadable(), moved here
🟨 api/knowledge/folders.php its copy removed; uses the shared one
🟧 api/v1/resources/knowledge.php create, update and restore pass $apiKey to apiLoadArticle()
πŸŸͺ tests/security-report-2026-10-part2.php section 8

Tested

tests/security-report-2026-10-part2.php, section 8, against the real database, with a temporary folder, article and two analysts it removes afterwards. The folder is given an access list naming one analyst (the owner); the other (denied) is left off.

  • Setup holds: the denied analyst cannot read the article; the owner can. πŸ”‘ The second line is the positive control β€” without it, a broken access list that denied everybody would make every refusal below look like a pass.
  • Every write is refused for the denied analyst, by calling the service directly: edit by id, archive, restore and purge each fail with not_found β€” the same code as a missing article.
  • Nothing changed: the article's title and archived flag are read back from the database afterwards, untouched.
  • No filing into an unreadable folder: a new article naming that folder fails with invalid_field, and no article with that title exists afterwards.
  • POSITIVE CONTROL: the owner may archive the same article β€” so the refusals are the access rule, not a broken service.
  • REST: the test reads api/v1/resources/knowledge.php and checks that create, update and restore all pass $apiKey to apiLoadArticle().

Do I need to do anything?

Upgrade. If you use restricted folders or articles and want to check nothing was changed by somebody who should not have, Knowledge writes a history row for every article edit (naming the fields that changed β€” folder and audience among them), archive, restore and purge, with the analyst who did it.

For developers

  • A write must pass the read check, and then its own. If someone cannot see a thing, they cannot change it. Start every write guard from the function that decides reads β€” here knowledgeCanRead() β€” rather than from a narrower one that happens to have the right name.
  • When a new kind of permission arrives, revisit every guard named for the old one. assertCanAccessArticle() was correct when "access" meant "company". Folder permissions changed what the word meant; the function did not change with it.
  • One rule, one copy. If a check you need exists as a private helper in an endpoint, move it into includes/ and call it from both places. Do not write a second one.
  • Refuse in the same words as "missing". not_found for an article, "not a known folder" for a folder.
  • Pass the caller to the loader. A loader with an optional "who is asking" argument is only as safe as its least careful caller. If you call apiLoadArticle(), pass the key.

Related: Knowledge folders and security β€” developer guide Β· Knowledge folders and permissions Β· Knowledge articles could run scripts Β· Company boundaries β€” seven places they did not hold Β· Bugs resolved

FreeITSM

Getting Started

Modules

Multi-tenancy (planned)

Blue sky thinking

Bugs resolved

Links

Clone this wiki locally