mirror of
https://github.com/snipe/snipe-it.git
synced 2026-08-18 11:15:42 +00:00
Ficed FD-56812 - better escaping for EULAs
This commit is contained in:
@ -206,24 +206,68 @@ class SnipeModel extends Model
|
||||
|
||||
public function getEula()
|
||||
{
|
||||
// Resolve the raw eula text from the appropriate source, then hand
|
||||
// it to sanitizeEulaForRender before returning. See that method for
|
||||
// the security rationale behind the sanitize step.
|
||||
$raw = null;
|
||||
|
||||
// This is - for now - only for assets, where the asset model is the thing tied to the category
|
||||
if (($this->model) && ($this->model->category)) {
|
||||
if (($this->model->category->eula_text) && ($this->model->category->use_default_eula == 0)) {
|
||||
return $this->model->category->eula_text;
|
||||
$raw = $this->model->category->eula_text;
|
||||
} elseif ($this->model->category->use_default_eula == 1) {
|
||||
return Setting::getSettings()->default_eula_text;
|
||||
$raw = Setting::getSettings()->default_eula_text;
|
||||
} else {
|
||||
return false;
|
||||
}
|
||||
// For everything else, just check the category for EULA info
|
||||
} elseif (($this->category) && ($this->category->eula_text)) {
|
||||
return $this->category->eula_text;
|
||||
$raw = $this->category->eula_text;
|
||||
} elseif ((Setting::getSettings()->default_eula_text) && (($this->category) && ($this->category->use_default_eula == '1'))) {
|
||||
return Setting::getSettings()->default_eula_text;
|
||||
$raw = Setting::getSettings()->default_eula_text;
|
||||
}
|
||||
|
||||
return null;
|
||||
return $this->sanitizeEulaForRender($raw);
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize raw eula_text before it lands in any renderer. This method
|
||||
* is invoked by getEula and mirrors the shape Category::getEula uses on
|
||||
* the web path (Helper::parseEscapedMarkedown = strip_tags + Parsedown
|
||||
* safe mode) with one addition: an <img> strip on the Parsedown output.
|
||||
*
|
||||
* The extra <img> strip is what closes the LFR + SSRF primitive reported
|
||||
* by W1nterFr3ak (Chris Byron Otieno) on 2026-08-02. Every checkout mail
|
||||
* template embeds this via `{!! $eula !!}` into a Markdown mailable,
|
||||
* whose HTML output is walked by laravel-mail-auto-embed, which fetches
|
||||
* every <img src=""> server-side (file_get_contents for local paths,
|
||||
* curl with TLS verification disabled for remote URLs) and attaches the
|
||||
* bytes to the outgoing mail. Any low-privilege user with categories.edit
|
||||
* could set eula_text to `` or a raw <img> tag,
|
||||
* check the asset out to themselves, and receive the file contents (or
|
||||
* the response body of any URL, including cloud instance metadata) as
|
||||
* a MIME attachment.
|
||||
*
|
||||
* strip_tags kills raw <img> HTML the user might have typed directly.
|
||||
* Parsedown safe mode converts markdown to HTML. The second img-strip
|
||||
* removes markdown-syntax images that Parsedown converted
|
||||
* (e.g. `` becoming `<img src=url>`). BlockImagesMarkdownExtension
|
||||
* on the mail Markdown parser (see config/mail.php) is defense in depth
|
||||
* for anything that slips past this pre-sanitize.
|
||||
*/
|
||||
protected function sanitizeEulaForRender(?string $raw): ?string
|
||||
{
|
||||
if ($raw === null || $raw === '') {
|
||||
return null;
|
||||
}
|
||||
|
||||
$rendered = Helper::parseEscapedMarkedown($raw);
|
||||
|
||||
if ($rendered === null || $rendered === '') {
|
||||
return null;
|
||||
}
|
||||
|
||||
return preg_replace('/<img\b[^>]*>/i', '', $rendered);
|
||||
}
|
||||
|
||||
public function getImageUrl($path = null)
|
||||
|
||||
@ -68,7 +68,12 @@
|
||||
|
||||
@if (!$singular_eula && $group->first()->eula)
|
||||
<hr>
|
||||
{{ $group->first()->eula }}
|
||||
{{-- eula is pre-sanitized by SnipeModel::getEula (strip_tags + Parsedown
|
||||
safe mode + <img> strip) before being loaded into $asset->eula in
|
||||
BulkAssetCheckoutMail::content, so emitting the resulting HTML raw
|
||||
preserves formatting without reintroducing the mail-auto-embed
|
||||
LFR/SSRF vector. --}}
|
||||
{!! $group->first()->eula !!}
|
||||
@endif
|
||||
|
||||
</x-mail::panel>
|
||||
@ -76,7 +81,7 @@
|
||||
|
||||
@if ($singular_eula)
|
||||
<x-mail::panel>
|
||||
{{ $singular_eula }}
|
||||
{!! $singular_eula !!}
|
||||
</x-mail::panel>
|
||||
@endif
|
||||
|
||||
|
||||
@ -0,0 +1,139 @@
|
||||
<?php
|
||||
|
||||
namespace Tests\Feature\CheckoutAcceptances;
|
||||
|
||||
use App\Mail\CheckoutAssetMail;
|
||||
use App\Models\Asset;
|
||||
use App\Models\AssetModel;
|
||||
use App\Models\Category;
|
||||
use App\Models\User;
|
||||
use Illuminate\Support\Facades\Mail;
|
||||
use Tests\TestCase;
|
||||
|
||||
/**
|
||||
* Regression coverage for the arbitrary local-file read + SSRF reported by
|
||||
* W1nterFr3ak (Chris Byron Otieno) on 2026-08-02. Category eula_text was
|
||||
* passed raw through SnipeModel::getEula, echoed via `{!! $eula !!}` into
|
||||
* every checkout mail template, and the resulting HTML was walked by
|
||||
* `laravel-mail-auto-embed`, which fetched every `<img src="">`
|
||||
* server-side (file_get_contents for local paths, curl with TLS
|
||||
* verification disabled for remote URLs) and attached the bytes to the
|
||||
* outbound mail. A low-privilege user with categories.create/edit +
|
||||
* assets.checkout could set eula_text to `` or a
|
||||
* raw `<img>` tag, check the asset out to themselves, and receive the
|
||||
* file contents (or any URL's response body, including cloud metadata) as
|
||||
* a MIME attachment.
|
||||
*
|
||||
* The fix sanitizes at the model boundary: `SnipeModel::getEula` now
|
||||
* pipes through `Helper::parseEscapedMarkedown` (strip_tags + Parsedown
|
||||
* safe mode) and additionally strips `<img>` from the Parsedown output,
|
||||
* killing both attack vectors before eula content reaches any mail
|
||||
* template. `BlockImagesMarkdownExtension` on the mail Markdown parser
|
||||
* from GHSA-f3vq-g24v-xc2g remains defense in depth.
|
||||
*
|
||||
* These tests exercise the model-layer sanitizer directly and the
|
||||
* end-to-end mailable render so both surfaces are pinned.
|
||||
*/
|
||||
class EulaMailAutoEmbedInjectionTest extends TestCase
|
||||
{
|
||||
private function assetWithEula(string $eulaText): Asset
|
||||
{
|
||||
$category = Category::factory()->assetLaptopCategory()->create([
|
||||
'eula_text' => $eulaText,
|
||||
'use_default_eula' => 0,
|
||||
]);
|
||||
$model = AssetModel::factory()->create(['category_id' => $category->id]);
|
||||
|
||||
return Asset::factory()->create(['model_id' => $model->id]);
|
||||
}
|
||||
|
||||
public function test_get_eula_strips_markdown_syntax_image_pointing_at_local_file()
|
||||
{
|
||||
$asset = $this->assetWithEula('');
|
||||
|
||||
$rendered = $asset->getEula();
|
||||
|
||||
$this->assertStringNotContainsString('<img', (string) $rendered);
|
||||
$this->assertStringNotContainsString('/var/www/html/.env', (string) $rendered);
|
||||
}
|
||||
|
||||
public function test_get_eula_strips_raw_html_img_pointing_at_local_file()
|
||||
{
|
||||
$asset = $this->assetWithEula('<img src="/var/www/html/.env" alt="logo">');
|
||||
|
||||
$rendered = $asset->getEula();
|
||||
|
||||
$this->assertStringNotContainsString('<img', (string) $rendered);
|
||||
$this->assertStringNotContainsString('/var/www/html/.env', (string) $rendered);
|
||||
}
|
||||
|
||||
public function test_get_eula_strips_markdown_syntax_image_pointing_at_ssrf_target()
|
||||
{
|
||||
$asset = $this->assetWithEula('');
|
||||
|
||||
$rendered = $asset->getEula();
|
||||
|
||||
$this->assertStringNotContainsString('<img', (string) $rendered);
|
||||
$this->assertStringNotContainsString('169.254.169.254', (string) $rendered);
|
||||
}
|
||||
|
||||
public function test_get_eula_strips_raw_html_img_pointing_at_loopback_ssrf_target()
|
||||
{
|
||||
$asset = $this->assetWithEula('<img src="http://127.0.0.1:9999/secret" alt="ssrf">');
|
||||
|
||||
$rendered = $asset->getEula();
|
||||
|
||||
$this->assertStringNotContainsString('<img', (string) $rendered);
|
||||
$this->assertStringNotContainsString('127.0.0.1', (string) $rendered);
|
||||
}
|
||||
|
||||
public function test_get_eula_preserves_legitimate_markdown_formatting()
|
||||
{
|
||||
$asset = $this->assetWithEula("**Terms** apply.\n\n- item one\n- item two");
|
||||
|
||||
$rendered = (string) $asset->getEula();
|
||||
|
||||
$this->assertStringContainsString('<strong>Terms</strong>', $rendered);
|
||||
$this->assertStringContainsString('<li>item one</li>', $rendered);
|
||||
}
|
||||
|
||||
public function test_checkout_asset_mail_render_omits_poisoned_img_from_eula()
|
||||
{
|
||||
$asset = $this->assetWithEula('');
|
||||
$target = User::factory()->create();
|
||||
$admin = User::factory()->create();
|
||||
|
||||
$mail = new CheckoutAssetMail(
|
||||
$asset,
|
||||
$target,
|
||||
$admin,
|
||||
null,
|
||||
null,
|
||||
);
|
||||
|
||||
$rendered = (string) $mail->render();
|
||||
|
||||
$this->assertStringNotContainsString('<img', $rendered);
|
||||
$this->assertStringNotContainsString('/var/www/html/.env', $rendered);
|
||||
}
|
||||
|
||||
public function test_checkout_asset_mail_render_omits_raw_html_img_from_eula()
|
||||
{
|
||||
$asset = $this->assetWithEula('<img src="/etc/hostname" alt="logo">');
|
||||
$target = User::factory()->create();
|
||||
$admin = User::factory()->create();
|
||||
|
||||
$mail = new CheckoutAssetMail(
|
||||
$asset,
|
||||
$target,
|
||||
$admin,
|
||||
null,
|
||||
null,
|
||||
);
|
||||
|
||||
$rendered = (string) $mail->render();
|
||||
|
||||
$this->assertStringNotContainsString('<img', $rendered);
|
||||
$this->assertStringNotContainsString('/etc/hostname', $rendered);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user