Skip to content
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2b7c0ac
fix: validate file path in Blueprints::import() to prevent SSRF/LFI
javokhir-sec Jul 3, 2026
7e8bf7f
fix(blueprints): harden import() path validation against SSRF/LFI
javokhir-sec Jul 20, 2026
42bd4e5
fix(blueprints): address Copilot review — TOCTOU, log level, test ass…
javokhir-sec Jul 20, 2026
112279f
fix(test): correct XML canvas key to match getDatabaseType()
javokhir-sec Jul 21, 2026
20823a9
fix(test): use correct box name 'problem' from lean canvas template
javokhir-sec Jul 21, 2026
c594b4f
style: fix pint formatting
javokhir-sec Jul 21, 2026
ed54874
style: fix pint formatting
javokhir-sec Jul 21, 2026
4b0d1d7
fix(test): apply Copilot review — portability, unique names, cleanup
javokhir-sec Jul 21, 2026
d7d6ca9
Merge branch 'fix/ssrf-lfi-final' into fix/pint-csrf-final
javokhir-sec Jul 21, 2026
25645b9
chore: remove accidentally added files
javokhir-sec Jul 21, 2026
08b82db
fix: use base_path() for userfiles + fix tempnam orphan file
javokhir-sec Jul 21, 2026
8d8f629
fix(test): replace tempnam with rename to avoid orphaned temp files
javokhir-sec Jul 21, 2026
afe9f27
Merge branch 'fix/ssrf-lfi-final' into fix/pint-csrf-final
javokhir-sec Jul 21, 2026
c7b5370
fix(test): proper LFI test using .xml outside allowed dirs
javokhir-sec Jul 21, 2026
5fceadd
Merge branch 'fix/ssrf-lfi-final' into fix/pint-csrf-final
javokhir-sec Jul 21, 2026
e19ae8c
fix: remove composer.phar binary and add to .gitignore
javokhir-sec Jul 23, 2026
b280394
style: apply pint fixes — imports, concat_space, class_definition
javokhir-sec Jul 23, 2026
15a1652
test: add file:// wrapper assertion to SSRF URL test
javokhir-sec Jul 23, 2026
d849517
fix: sync all SSRF/LFI hardening fixes from PR #3656
javokhir-sec Jul 23, 2026
832192e
docs: clarify sys_get_temp_dir() trust model, restore .gitkeep
javokhir-sec Jul 23, 2026
c3686cb
fix: import and stub UserRepository in import acceptance test
javokhir-sec Jul 23, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,4 @@ tests/Httprequests/http-client.private.env.json
.claude/docs/personal-companion-requirements.md
.claude/docs/git-workflow.md
cleanup-architecture.md
composer.phar
77 changes: 76 additions & 1 deletion app/Domain/Blueprints/Services/Blueprints.php
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,64 @@ public function deleteBoard(int $canvasId, string $canvasType): void
$this->blueprintsRepo->deleteCanvas($canvasId);
}

/**
* Validate that a resolved import file path is safe to read.
*
* Rejects files outside a fixed allow-list of local directories and
* requires a known extension. The caller must resolve the path via
* {@see realpath()} first — this avoids a TOCTOU window between
* resolution and validation.
*
* The allow-list covers the three places Leantime sources blueprint
* import files from: the upload temp directory, the userfiles storage,
* and the shipped fixture directory under the Blueprints domain.
*
* @param string $resolvedPath Already-resolved absolute path (from realpath)
* @return bool True when the path is within an allowed directory
* and has an allowed extension
*/
private function isImportPathAllowed(string $resolvedPath): bool
{
$allowedDirs = [
sys_get_temp_dir(),
base_path('userfiles'),
APP_ROOT.'/app/Domain/Blueprints/imports',
];
Comment on lines +415 to +418
Comment on lines +415 to +418

// Validate file extension — only XML (uploaded via the UI) and JSON
// (shipped fixture files under the imports directory) are permitted.
$ext = strtolower(pathinfo($resolvedPath, PATHINFO_EXTENSION));
if (! in_array($ext, ['xml', 'json'], true)) {
Log::warning('Blueprints import: disallowed file extension', [
'resolvedPath' => $resolvedPath,
'extension' => $ext,
]);

return false;
}

// Anchor each allowed directory with a trailing separator so
// str_starts_with doesn't match sibling-prefix paths (e.g.
// /tmp-evil/x must NOT match against allowed /tmp).
foreach ($allowedDirs as $allowedDir) {
$resolvedAllowed = realpath($allowedDir);

if ($resolvedAllowed === false) {
continue;
}

if (str_starts_with($resolvedPath, $resolvedAllowed.DIRECTORY_SEPARATOR)) {
return true;
}
}
Comment on lines +436 to +446

Log::warning('Blueprints import: path traversal or SSRF attempt blocked', [
'resolvedPath' => $resolvedPath,
]);

return false;
}

/**
* Import a canvas board from an XML file.
*
Expand Down Expand Up @@ -412,7 +470,24 @@ public function import(string $filename, string $canvasSlug, int $projectId, int
$dom = new DOMDocument('1.0', 'UTF-8');
$users = app()->make(UserRepository::class);

Comment on lines 486 to 488
$canvasData = file_get_contents($filename);
// Validate the file path and extension to prevent SSRF and Local File
// Inclusion. Reject URL wrappers (http://, ftp://, etc.), restrict
// reads to allowed local directories, and require a known import
// extension.
$resolvedPath = realpath($filename);
Comment on lines +489 to +493
if ($resolvedPath === false) {
Log::warning('Blueprints import: file not found or path does not exist', [
'filename' => $filename,
]);

return false;
}

if (! $this->isImportPathAllowed($resolvedPath)) {
return false;
}

$canvasData = file_get_contents($resolvedPath);
if ($canvasData === false) {
return false;
}
Comment on lines +493 to 520
Expand Down
192 changes: 190 additions & 2 deletions tests/Unit/app/Domain/Blueprints/Services/BlueprintsServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ private function service(?BlueprintsRepository $repo = null, ?TemplateRegistry $
public function test_translated_boxes_run_titles_through_language(): void
{
$template = new CanvasTemplate([
'slug' => 'swot',
'slug' => 'lean',
'boxes' => ['swot_strengths' => ['icon' => 'fa-x', 'title' => 'box.swot.strengths']],
Comment on lines 44 to 46
]);

Expand Down Expand Up @@ -505,7 +505,195 @@ public function test_import_authorizes_create_on_target_project_before_doing_any
$service = $this->securedService($repo, $this->denyingPermissions());

$this->expectException(AuthorizationException::class);
$service->import('/tmp/does-not-matter.xml', 'swot', 55, 1);
$service->import('/tmp/does-not-matter.xml', 'lean', 55, 1);
}

// ---------------------------------------------------------------------
// import() path-validation regression tests (SSRF / LFI / CWE-918).
// ---------------------------------------------------------------------

public function test_import_rejects_ssrf_url_wrappers(): void
{
// URL wrappers such as http://, ftp:// resolve to false via realpath(),
// but even if/when a stream wrapper could produce a realpath, the
// allow-list check catches it. This test also guards the more
// subtle case of file:///etc/passwd which some PHP builds resolve.
$service = $this->securedService(
$this->make(BlueprintsRepository::class),
$this->allowingPermissions()
);

// SSRF: HTTP URL — realpath() returns false, caught as "file not found".
$this->assertFalse(
$service->import('http://169.254.169.254/latest/meta-data/', 'lean', 55, 1),
'HTTP URL must be rejected'
);

// SSRF: FTP URL.
$this->assertFalse(
$service->import('ftp://evil.com/blueprint.xml', 'lean', 55, 1),
'FTP URL must be rejected'
);
}

public function test_import_rejects_lfi_absolute_path_to_system_file(): void
{
// Create an .xml file in a directory that is NOT in the allowed list.
// /var/tmp is outside sys_temp_dir, userfiles, and Blueprints/imports.
$service = $this->securedService(
$this->make(BlueprintsRepository::class),
$this->allowingPermissions()
);

$outOfBounds = '/var/tmp/leantime_lfi_test_' . uniqid('', true) . '.xml';
file_put_contents($outOfBounds, '<canvas key="leancanvas"><title>LFI Test</title></canvas>');

try {
$this->assertFalse(
$service->import($outOfBounds, 'lean', 55, 1),
'Absolute path to an .xml file outside allowed directories must be rejected'
);
} finally {
if (file_exists($outOfBounds)) {
unlink($outOfBounds);
}
}
}
Comment on lines +550 to +574

public function test_import_rejects_dot_dot_path_traversal(): void
{
// Construct a path that starts inside the temp directory but
// traverses out and back into /etc/passwd.
$service = $this->securedService(
$this->make(BlueprintsRepository::class),
$this->allowingPermissions()
);

$traversal = sys_get_temp_dir().'/../../../etc/passwd';

$this->assertFalse(
$service->import($traversal, 'lean', 55, 1),
'Path traversal (../) must be rejected'
);
}

public function test_import_rejects_sibling_prefix_bypass(): void
{
// str_starts_with without separator anchoring would allow
// /tmp-evil/blueprint.xml to match against allowed /tmp.
// The fix appends DIRECTORY_SEPARATOR to each allowed dir.
$service = $this->securedService(
$this->make(BlueprintsRepository::class),
$this->allowingPermissions()
);

// Create a directory whose name is a prefix of the real temp dir.
$siblingDir = sys_get_temp_dir().'-evil';
if (! is_dir($siblingDir)) {
mkdir($siblingDir, 0700, true);
}
Comment on lines +603 to +609
$siblingFile = $siblingDir.'/blueprint.json';
file_put_contents($siblingFile, '{}');

try {
$this->assertFalse(
$service->import($siblingFile, 'lean', 55, 1),
'Sibling-prefix path (e.g. /tmp-evil/…) must NOT match allowed /tmp'
);
} finally {
unlink($siblingFile);
rmdir($siblingDir);
}
}

public function test_import_rejects_disallowed_file_extensions(): void
{
// Only .xml is permitted. Other extensions must be
// rejected even when the file sits in an allowed directory.
$service = $this->securedService(
$this->make(BlueprintsRepository::class),
$this->allowingPermissions()
);

$phpFile = sys_get_temp_dir().'/leantime_import_test.php';
file_put_contents($phpFile, '<?php echo "pwned";');

$txtFile = sys_get_temp_dir().'/leantime_import_test.txt';
file_put_contents($txtFile, 'not xml');

try {
$this->assertFalse(
$service->import($phpFile, 'lean', 55, 1),
'.php extension must be rejected in an allowed directory'
);
$this->assertFalse(
$service->import($txtFile, 'lean', 55, 1),
'.txt extension must be rejected in an allowed directory'
);
} finally {
if (file_exists($phpFile)) {
unlink($phpFile);
}
if (file_exists($txtFile)) {
unlink($txtFile);
}
}
}

public function test_import_accepts_xml_file_in_allowed_temp_dir(): void
{
// A .xml file placed in sys_get_temp_dir() (the normal upload flow)
// must pass path validation and successfully import via the repo.
// The repository is stubbed so the import completes and returns a
// known canvas id, proving that path validation did NOT block it.
$expectedId = 42;
$repo = $this->make(BlueprintsRepository::class, [
'existCanvas' => fn () => false,
'addCanvas' => fn () => $expectedId,
'addCanvasItem' => fn () => 1,
]);
$service = $this->securedService($repo, $this->allowingPermissions());

Comment on lines +671 to +678
$xml = <<<'XML'
<?xml version="1.0" encoding="UTF-8"?>
<canvas key="leancanvas">
<title>Security Test Canvas</title>
<content>
<element key="problem">
<item>
<author firstname="A" lastname="B"/>
<description>Test item</description>
<status>status_draft</status>
<relates>relates_none</relates>
<assumptions>none</assumptions>
<priority>low</priority>
<progress>0</progress>
<duedate></duedate>
<tags></tags>
</item>
</element>
</content>
</canvas>
XML;

$tmpBase = tempnam(sys_get_temp_dir(), 'leantime.');
$tempFile = $tmpBase . '.xml';
rename($tmpBase, $tempFile);
file_put_contents($tempFile, $xml);

try {
$result = $service->import($tempFile, 'lean', 55, 1);
// Path validation passed and repo returned the expected canvas id.
$this->assertSame(
$expectedId,
$result,
'XML file in allowed dir must pass path validation and be imported'
);
} finally {
if (file_exists($tempFile)) {
unlink($tempFile);
}
}
}

public function test_create_board_applies_start_content_against_the_slug_not_the_db_type(): void
Expand Down
Loading