From 8717f469b10e9f7e1547c6f70f7d24e1359d28d4 Mon Sep 17 00:00:00 2001
From: James Cole <thegrumpydictator@gmail.com>
Date: Sat, 3 Aug 2019 05:08:20 +0200
Subject: [PATCH 1/3] Fix #2370
---
app/Support/Http/Controllers/UserNavigation.php | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/app/Support/Http/Controllers/UserNavigation.php b/app/Support/Http/Controllers/UserNavigation.php
index 66e30c5c02..1cac4ed6b0 100644
--- a/app/Support/Http/Controllers/UserNavigation.php
+++ b/app/Support/Http/Controllers/UserNavigation.php
@@ -124,7 +124,7 @@ trait UserNavigation
/** @var Transaction $transaction */
$transaction = $account->transactions()->first();
if (null === $transaction) {
- app('session')->flash('error', trans('firefly.account_missing_transaction', ['name' => $account->name, 'id' => $account->id]));
+ app('session')->flash('error', trans('firefly.account_missing_transaction', ['name' => e($account->name), 'id' => $account->id]));
Log::error(sprintf('Expected a transaction. Account #%d has none. BEEP, error.', $account->id));
return redirect(route('index'));
@@ -135,7 +135,7 @@ trait UserNavigation
$opposingTransaction = $journal->transactions()->where('transactions.id', '!=', $transaction->id)->first();
if (null === $opposingTransaction) {
- app('session')->flash('error', trans('firefly.account_missing_transaction', ['name' => $account->name, 'id' => $account->id]));
+ app('session')->flash('error', trans('firefly.account_missing_transaction', ['name' => e($account->name), 'id' => $account->id]));
Log::error(sprintf('Expected an opposing transaction. Account #%d has none. BEEP, error.', $account->id));
}
From 2d7494f8cd53942b661774c9aabf3a0a9cf4053c Mon Sep 17 00:00:00 2001
From: James Cole <thegrumpydictator@gmail.com>
Date: Sat, 3 Aug 2019 05:08:35 +0200
Subject: [PATCH 2/3] Fix similar XSS issues.
---
app/Http/Controllers/Admin/LinkController.php | 6 +++---
app/Http/Controllers/CurrencyController.php | 18 +++++++++---------
.../Import/JobConfigurationController.php | 4 ++--
.../Import/PrerequisitesController.php | 6 +++---
app/Http/Controllers/PiggyBankController.php | 4 ++--
app/Http/Controllers/ProfileController.php | 10 +++++-----
.../Transaction/SingleController.php | 4 ++--
.../views/v1/form/assetAccountCheckList.twig | 2 +-
.../views/v1/import/bunq/choose-accounts.twig | 2 +-
9 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/app/Http/Controllers/Admin/LinkController.php b/app/Http/Controllers/Admin/LinkController.php
index 4a3a3afcea..1188f28de3 100644
--- a/app/Http/Controllers/Admin/LinkController.php
+++ b/app/Http/Controllers/Admin/LinkController.php
@@ -86,7 +86,7 @@ class LinkController extends Controller
public function delete(Request $request, LinkTypeRepositoryInterface $repository, LinkType $linkType)
{
if (!$linkType->editable) {
- $request->session()->flash('error', (string)trans('firefly.cannot_edit_link_type', ['name' => $linkType->name]));
+ $request->session()->flash('error', (string)trans('firefly.cannot_edit_link_type', ['name' => e($linkType->name)]));
return redirect(route('admin.links.index'));
}
@@ -143,7 +143,7 @@ class LinkController extends Controller
public function edit(Request $request, LinkType $linkType)
{
if (!$linkType->editable) {
- $request->session()->flash('error', (string)trans('firefly.cannot_edit_link_type', ['name' => $linkType->name]));
+ $request->session()->flash('error', (string)trans('firefly.cannot_edit_link_type', ['name' => e($linkType->name)]));
return redirect(route('admin.links.index'));
}
@@ -246,7 +246,7 @@ class LinkController extends Controller
public function update(LinkTypeFormRequest $request, LinkTypeRepositoryInterface $repository, LinkType $linkType)
{
if (!$linkType->editable) {
- $request->session()->flash('error', (string)trans('firefly.cannot_edit_link_type', ['name' => $linkType->name]));
+ $request->session()->flash('error', (string)trans('firefly.cannot_edit_link_type', ['name' => e($linkType->name)]));
return redirect(route('admin.links.index'));
}
diff --git a/app/Http/Controllers/CurrencyController.php b/app/Http/Controllers/CurrencyController.php
index 45d932919b..ee89e3b4f7 100644
--- a/app/Http/Controllers/CurrencyController.php
+++ b/app/Http/Controllers/CurrencyController.php
@@ -77,7 +77,7 @@ class CurrencyController extends Controller
/** @var User $user */
$user = auth()->user();
if (!$this->userRepository->hasRole($user, 'owner')) {
- $request->session()->flash('error', (string)trans('firefly.ask_site_owner', ['owner' => config('firefly.site_owner')]));
+ $request->session()->flash('error', (string)trans('firefly.ask_site_owner', ['owner' => e(config('firefly.site_owner'))]));
return redirect(route('currencies.index'));
}
@@ -131,7 +131,7 @@ class CurrencyController extends Controller
$user = auth()->user();
if (!$this->userRepository->hasRole($user, 'owner')) {
// @codeCoverageIgnoreStart
- $request->session()->flash('error', (string)trans('firefly.ask_site_owner', ['owner' => config('firefly.site_owner')]));
+ $request->session()->flash('error', (string)trans('firefly.ask_site_owner', ['owner' => e(config('firefly.site_owner'))]));
Log::channel('audit')->info(sprintf('Tried to visit page to delete currency %s but is not site owner.', $currency->code));
return redirect(route('currencies.index'));
@@ -139,7 +139,7 @@ class CurrencyController extends Controller
}
if ($this->repository->currencyInUse($currency)) {
- $request->session()->flash('error', (string)trans('firefly.cannot_delete_currency', ['name' => $currency->name]));
+ $request->session()->flash('error', (string)trans('firefly.cannot_delete_currency', ['name' => e($currency->name)]));
Log::channel('audit')->info(sprintf('Tried to visit page to delete currency %s but currency is in use.', $currency->code));
return redirect(route('currencies.index'));
@@ -167,7 +167,7 @@ class CurrencyController extends Controller
$user = auth()->user();
if (!$this->userRepository->hasRole($user, 'owner')) {
// @codeCoverageIgnoreStart
- $request->session()->flash('error', (string)trans('firefly.ask_site_owner', ['owner' => config('firefly.site_owner')]));
+ $request->session()->flash('error', (string)trans('firefly.ask_site_owner', ['owner' => e(config('firefly.site_owner'))]));
Log::channel('audit')->info(sprintf('Tried to delete currency %s but is not site owner.', $currency->code));
return redirect(route('currencies.index'));
@@ -175,7 +175,7 @@ class CurrencyController extends Controller
}
if ($this->repository->currencyInUse($currency)) {
- $request->session()->flash('error', (string)trans('firefly.cannot_delete_currency', ['name' => $currency->name]));
+ $request->session()->flash('error', (string)trans('firefly.cannot_delete_currency', ['name' => e($currency->name)]));
Log::channel('audit')->info(sprintf('Tried to delete currency %s but is in use.', $currency->code));
return redirect(route('currencies.index'));
@@ -203,7 +203,7 @@ class CurrencyController extends Controller
$user = auth()->user();
if (!$this->userRepository->hasRole($user, 'owner')) {
// @codeCoverageIgnoreStart
- $request->session()->flash('error', (string)trans('firefly.ask_site_owner', ['owner' => config('firefly.site_owner')]));
+ $request->session()->flash('error', (string)trans('firefly.ask_site_owner', ['owner' => e(config('firefly.site_owner'))]));
Log::channel('audit')->info(sprintf('Tried to disable currency %s but is not site owner.', $currency->code));
return redirect(route('currencies.index'));
@@ -211,7 +211,7 @@ class CurrencyController extends Controller
}
if ($this->repository->currencyInUse($currency)) {
- $request->session()->flash('error', (string)trans('firefly.cannot_disable_currency', ['name' => $currency->name]));
+ $request->session()->flash('error', (string)trans('firefly.cannot_disable_currency', ['name' => e($currency->name)]));
Log::channel('audit')->info(sprintf('Tried to disable currency %s but is in use.', $currency->code));
return redirect(route('currencies.index'));
@@ -251,7 +251,7 @@ class CurrencyController extends Controller
$user = auth()->user();
if (!$this->userRepository->hasRole($user, 'owner')) {
// @codeCoverageIgnoreStart
- $request->session()->flash('error', (string)trans('firefly.ask_site_owner', ['owner' => config('firefly.site_owner')]));
+ $request->session()->flash('error', (string)trans('firefly.ask_site_owner', ['owner' => e(config('firefly.site_owner'))]));
Log::channel('audit')->info(sprintf('Tried to edit currency %s but is not owner.', $currency->code));
return redirect(route('currencies.index'));
@@ -395,7 +395,7 @@ class CurrencyController extends Controller
$data = $request->getCurrencyData();
if (!$this->userRepository->hasRole($user, 'owner')) {
// @codeCoverageIgnoreStart
- $request->session()->flash('error', (string)trans('firefly.ask_site_owner', ['owner' => config('firefly.site_owner')]));
+ $request->session()->flash('error', (string)trans('firefly.ask_site_owner', ['owner' => e(config('firefly.site_owner'))]));
Log::channel('audit')->info('Tried to update (POST) currency without admin rights.', $data);
return redirect(route('currencies.index'));
diff --git a/app/Http/Controllers/Import/JobConfigurationController.php b/app/Http/Controllers/Import/JobConfigurationController.php
index 30460f3dea..20a7f0966a 100644
--- a/app/Http/Controllers/Import/JobConfigurationController.php
+++ b/app/Http/Controllers/Import/JobConfigurationController.php
@@ -77,7 +77,7 @@ class JobConfigurationController extends Controller
$allowed = ['has_prereq', 'need_job_config'];
if (null !== $importJob && !\in_array($importJob->status, $allowed, true)) {
Log::error(sprintf('Job has state "%s", but we only accept %s', $importJob->status, json_encode($allowed)));
- session()->flash('error', (string)trans('import.bad_job_status', ['status' => $importJob->status]));
+ session()->flash('error', (string)trans('import.bad_job_status', ['status' => e($importJob->status)]));
return redirect(route('import.index'));
}
@@ -127,7 +127,7 @@ class JobConfigurationController extends Controller
// catch impossible status:
$allowed = ['has_prereq', 'need_job_config'];
if (null !== $importJob && !\in_array($importJob->status, $allowed, true)) {
- session()->flash('error', (string)trans('import.bad_job_status', ['status' => $importJob->status]));
+ session()->flash('error', (string)trans('import.bad_job_status', ['status' => e($importJob->status)]));
return redirect(route('import.index'));
}
diff --git a/app/Http/Controllers/Import/PrerequisitesController.php b/app/Http/Controllers/Import/PrerequisitesController.php
index 74835d62f9..da9c5ba82c 100644
--- a/app/Http/Controllers/Import/PrerequisitesController.php
+++ b/app/Http/Controllers/Import/PrerequisitesController.php
@@ -76,7 +76,7 @@ class PrerequisitesController extends Controller
$allowed = ['new'];
if (null !== $importJob && !\in_array($importJob->status, $allowed, true)) {
Log::error(sprintf('Job has state "%s" but this Prerequisites::index() only accepts %s', $importJob->status, json_encode($allowed)));
- session()->flash('error', (string)trans('import.bad_job_status', ['status' => $importJob->status]));
+ session()->flash('error', (string)trans('import.bad_job_status', ['status' => e($importJob->status)]));
return redirect(route('import.index'));
}
@@ -129,7 +129,7 @@ class PrerequisitesController extends Controller
$allowed = ['new'];
if (null !== $importJob && !\in_array($importJob->status, $allowed, true)) {
Log::error(sprintf('Job has state "%s" but this Prerequisites::post() only accepts %s', $importJob->status, json_encode($allowed)));
- session()->flash('error', (string)trans('import.bad_job_status', ['status' => $importJob->status]));
+ session()->flash('error', (string)trans('import.bad_job_status', ['status' => e($importJob->status)]));
return redirect(route('import.index'));
}
@@ -148,7 +148,7 @@ class PrerequisitesController extends Controller
Log::debug(sprintf('Result of storePrerequisites has message count: %d', $result->count()));
if ($result->count() > 0) {
- $request->session()->flash('error', $result->first());
+ $request->session()->flash('error', e($result->first()));
// redirect back to job, if has job:
return redirect(route('import.prerequisites.index', [$importProvider, $importJob->key ?? '']))->withInput();
diff --git a/app/Http/Controllers/PiggyBankController.php b/app/Http/Controllers/PiggyBankController.php
index 6261a88888..412d6a0f88 100644
--- a/app/Http/Controllers/PiggyBankController.php
+++ b/app/Http/Controllers/PiggyBankController.php
@@ -320,7 +320,7 @@ class PiggyBankController extends Controller
'error',
(string)trans(
'firefly.cannot_add_amount_piggy',
- ['amount' => app('amount')->formatAnything($currency, $amount, false), 'name' => $piggyBank->name]
+ ['amount' => app('amount')->formatAnything($currency, $amount, false), 'name' => e($piggyBank->name)]
)
);
@@ -363,7 +363,7 @@ class PiggyBankController extends Controller
'error',
(string)trans(
'firefly.cannot_remove_from_piggy',
- ['amount' => app('amount')->formatAnything($currency, $amount, false), 'name' => $piggyBank->name]
+ ['amount' => app('amount')->formatAnything($currency, $amount, false), 'name' => e($piggyBank->name)]
)
);
diff --git a/app/Http/Controllers/ProfileController.php b/app/Http/Controllers/ProfileController.php
index 472255734d..2adbaf40c3 100644
--- a/app/Http/Controllers/ProfileController.php
+++ b/app/Http/Controllers/ProfileController.php
@@ -89,7 +89,7 @@ class ProfileController extends Controller
$loginProvider = config('firefly.login_provider');
if ('eloquent' !== $loginProvider) {
// @codeCoverageIgnoreStart
- $request->session()->flash('error', trans('firefly.login_provider_local_only', ['login_provider' => $loginProvider]));
+ $request->session()->flash('error', trans('firefly.login_provider_local_only', ['login_provider' => e($loginProvider)]));
return redirect(route('profile.index'));
// @codeCoverageIgnoreEnd
@@ -115,7 +115,7 @@ class ProfileController extends Controller
$loginProvider = config('firefly.login_provider');
if ('eloquent' !== $loginProvider) {
// @codeCoverageIgnoreStart
- $request->session()->flash('error', trans('firefly.login_provider_local_only', ['login_provider' => $loginProvider]));
+ $request->session()->flash('error', trans('firefly.login_provider_local_only', ['login_provider' => e($loginProvider)]));
return redirect(route('profile.index'));
// @codeCoverageIgnoreEnd
@@ -200,7 +200,7 @@ class ProfileController extends Controller
$loginProvider = config('firefly.login_provider');
if ('eloquent' !== $loginProvider) {
// @codeCoverageIgnoreStart
- $request->session()->flash('warning', trans('firefly.delete_local_info_only', ['login_provider' => $loginProvider]));
+ $request->session()->flash('warning', trans('firefly.delete_local_info_only', ['login_provider' => e($loginProvider)]));
// @codeCoverageIgnoreEnd
}
$title = auth()->user()->email;
@@ -296,7 +296,7 @@ class ProfileController extends Controller
$loginProvider = config('firefly.login_provider');
if ('eloquent' !== $loginProvider) {
// @codeCoverageIgnoreStart
- $request->session()->flash('error', trans('firefly.login_provider_local_only', ['login_provider' => $loginProvider]));
+ $request->session()->flash('error', trans('firefly.login_provider_local_only', ['login_provider' => e($loginProvider)]));
return redirect(route('profile.index'));
// @codeCoverageIgnoreEnd
@@ -350,7 +350,7 @@ class ProfileController extends Controller
$loginProvider = config('firefly.login_provider');
if ('eloquent' !== $loginProvider) {
// @codeCoverageIgnoreStart
- $request->session()->flash('error', trans('firefly.login_provider_local_only', ['login_provider' => $loginProvider]));
+ $request->session()->flash('error', trans('firefly.login_provider_local_only', ['login_provider' => e($loginProvider)]));
return redirect(route('profile.index'));
// @codeCoverageIgnoreEnd
diff --git a/app/Http/Controllers/Transaction/SingleController.php b/app/Http/Controllers/Transaction/SingleController.php
index bbf82c8737..50a44d616b 100644
--- a/app/Http/Controllers/Transaction/SingleController.php
+++ b/app/Http/Controllers/Transaction/SingleController.php
@@ -396,7 +396,7 @@ class SingleController extends Controller
// store the journal only, flash the rest.
Log::debug(sprintf('Count of error messages is %d', $this->attachments->getErrors()->count()));
if (\count($this->attachments->getErrors()->get('attachments')) > 0) {
- session()->flash('error', $this->attachments->getErrors()->get('attachments'));
+ session()->flash('error', e($this->attachments->getErrors()->get('attachments')));
}
// flash messages
if (\count($this->attachments->getMessages()->get('attachments')) > 0) {
@@ -463,7 +463,7 @@ class SingleController extends Controller
// @codeCoverageIgnoreStart
if (\count($this->attachments->getErrors()->get('attachments')) > 0) {
- session()->flash('error', $this->attachments->getErrors()->get('attachments'));
+ session()->flash('error', e($this->attachments->getErrors()->get('attachments')));
}
if (\count($this->attachments->getMessages()->get('attachments')) > 0) {
session()->flash('info', $this->attachments->getMessages()->get('attachments'));
diff --git a/resources/views/v1/form/assetAccountCheckList.twig b/resources/views/v1/form/assetAccountCheckList.twig
index 129dc11356..d47ea5d1e3 100644
--- a/resources/views/v1/form/assetAccountCheckList.twig
+++ b/resources/views/v1/form/assetAccountCheckList.twig
@@ -12,7 +12,7 @@
{% else %}
{{ Form.checkbox(name~'[]', id, false, options) }}
{% endif %}
- {{ account }}
+ {{ account|escape }}
</label>
</div>
{% endfor %}
diff --git a/resources/views/v1/import/bunq/choose-accounts.twig b/resources/views/v1/import/bunq/choose-accounts.twig
index 4d5f395f30..b43e6f3d48 100644
--- a/resources/views/v1/import/bunq/choose-accounts.twig
+++ b/resources/views/v1/import/bunq/choose-accounts.twig
@@ -64,7 +64,7 @@
{% endif %}
{% endfor %}
{% if account.status != 'ACTIVE' %}
- <li>{{ trans('import.bunq_account_status_'~account.status) }}</li>
+ <li>{{ trans('import.bunq_account_status_'~account.status|escape) }}</li>
{% endif %}
{% if account.type == 'MonetaryAccountSavings' %}
<li>{{ trans('import.bunq_savings_goal', {'amount': account.savingsGoal.currency ~' '~account.savingsGoal.value,'percentage' : account.savingsGoal.percentage}) }}</li>
From ec8e1d5a7a199843c9b0095213b94f0830656c09 Mon Sep 17 00:00:00 2001
From: James Cole <thegrumpydictator@gmail.com>
Date: Sat, 3 Aug 2019 05:10:06 +0200
Subject: [PATCH 3/3] Version update
---
.sandstorm/changelog.md | 4 ++++
.sandstorm/sandstorm-pkgdef.capnp | 4 ++--
.travis.yml | 2 +-
changelog.md | 5 +++++
config/firefly.php | 2 +-
5 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/.sandstorm/changelog.md b/.sandstorm/changelog.md
index 99ac9cb943..3e742969dd 100644
--- a/.sandstorm/changelog.md
+++ b/.sandstorm/changelog.md
@@ -1,3 +1,7 @@
+# 4.7.17.6 (API 0.9.2)
+
+- XSS issue in liability account redirect, found by [@0x2500](https://github.com/0x2500).
+
# 4.7.17.5 (API 0.9.2)
- Several XSS issues, found by [@0x2500](https://github.com/0x2500).
diff --git a/.sandstorm/sandstorm-pkgdef.capnp b/.sandstorm/sandstorm-pkgdef.capnp
index eae7372e28..ee3ce6388e 100644
--- a/.sandstorm/sandstorm-pkgdef.capnp
+++ b/.sandstorm/sandstorm-pkgdef.capnp
@@ -15,8 +15,8 @@ const pkgdef :Spk.PackageDefinition = (
manifest = (
appTitle = (defaultText = "Firefly III"),
- appVersion = 31,
- appMarketingVersion = (defaultText = "4.7.17.5"),
+ appVersion = 32,
+ appMarketingVersion = (defaultText = "4.7.17.6"),
actions = [
# Define your "new document" handlers here.
diff --git a/.travis.yml b/.travis.yml
index cc58593925..e7c74e3738 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,7 +1,7 @@
sudo: required
language: bash
env:
- - VERSION=4.7.17.5
+ - VERSION=4.7.17.6
dist: xenial
diff --git a/changelog.md b/changelog.md
index 01b7c9a969..47c7f4f1c3 100644
--- a/changelog.md
+++ b/changelog.md
@@ -2,6 +2,11 @@
All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).
+## [4.7.17.6 (API 0.9.2)] - 2019-08-02
+
+### Security
+- XSS issue in liability account redirect, found by [@0x2500](https://github.com/0x2500).
+
## [4.7.17.5 (API 0.9.2)] - 2019-08-02
### Security
diff --git a/config/firefly.php b/config/firefly.php
index 5592d289ca..e2efe34136 100644
--- a/config/firefly.php
+++ b/config/firefly.php
@@ -93,7 +93,7 @@ return [
'is_demo_site' => false,
],
'encryption' => null === env('USE_ENCRYPTION') || env('USE_ENCRYPTION') === true,
- 'version' => '4.7.17.5',
+ 'version' => '4.7.17.6',
'api_version' => '0.9.2',
'db_version' => 10,
'maxUploadSize' => 15242880,