CVE-2021-3728: Cross-Site Request Forgery (CSRF) (CWE-352)
value: firefly-iii is vulnerable to Cross-Site Request Forgery (CSRF)
CWE description: The web application does not, or can not, sufficiently verify whether a well-formed, valid, consistent request was intentionally provided by the user who submitted the request.
CVSS v3 base score: 6.5
Published: 2021-08-23T13:15Z
Repository: https://github.com/firefly-iii/firefly-iii
Language: PHP
Files changed: public/v1/js/ff/budgets/index.js, resources/views/v1/budgets/index.twig, routes/web.php
Commit date: 2021-08-20 09:38:52 +0200
Commit message: Fix vulnerability https://huntr.dev/bounties/dd54c5a1-0d4a-4f02-a111-7ce4ddc67a4d/

Diff: diff --git a/public/v1/js/ff/budgets/index.js b/public/v1/js/ff/budgets/index.js
index fcccb5cbec..0e8ab6f071 100644
--- a/public/v1/js/ff/budgets/index.js
+++ b/public/v1/js/ff/budgets/index.js
@@ -131,11 +131,11 @@ function updateTotalBudgetedAmount(currencyId) {
         .fadeTo(100, 0.1, function () {
             //$(this).fadeTo(500, 1.0);
         });
 
     // get new amount:
-    $.get(totalBudgetedUri.replace('REPLACEME',currencyId)).done(function (data) {
+    $.get(totalBudgetedUri.replace('REPLACEME', currencyId)).done(function (data) {
         // set thing:
         $('span.budgeted_amount[data-currency="' + currencyId + '"]')
             .html(data.budgeted_formatted)
             // fade back:
             .fadeTo(300, 1.0);
@@ -205,13 +205,18 @@ function createBudgetLimit(e) {
     });
     return false;
 }
 
 function deleteBudgetLimit(e) {
+    e.preventDefault();
     var button = $(e.currentTarget);
     var budgetLimitId = button.data('budget-limit-id');
-    var url = deleteBudgetLimitUrl.replace('REPLACEME', budgetId.toString();
+    var url = deleteBudgetLimitUrl.replace('REPLACEME', budgetLimitId.toString());
+    $.post(url, {_token: token}).then(function () {
+        $('.bl_entry[data-budget-limit-id="' + budgetLimitId + '"]').remove();
+        
+    });
     return false;
 }
 
 function createAltAvailableBudget(e) {
     $('#defaultModal').empty().load(createAltAvailableBudgetUri, function () {
diff --git a/resources/views/v1/budgets/index.twig b/resources/views/v1/budgets/index.twig
index 763525300f..43a63e8974 100644
--- a/resources/views/v1/budgets/index.twig
+++ b/resources/views/v1/budgets/index.twig
@@ -283,21 +283,23 @@
                                                 {% if not budgetLimit.in_range %}
                                                     <small class="text-muted">
                                                         {{ trans('firefly.budget_limit_not_in_range', {start: budgetLimit.start_date, end: budgetLimit.end_date}) }}
                                                     </small><br>
                                                 {% endif %}
-                                                <div class="input-group">
+                                                <div class="input-group bl_entry" data-budget-limit-id="{{ budgetLimit.id }}">
                                                     <div class="input-group-addon">{{ budgetLimit.currency_symbol }}</div>
                                                     <input class="form-control budget_amount" data-original="{{ budgetLimit.amount }}"
                                                         data-id="{{ budget.id }}" data-limit="{{ budgetLimit.id }}" value="{{ budgetLimit.amount }}"
                                                         autocomplete="off"
                                                         min="0" name="amount" type="number">
                                                     <div class="input-group-btn">
                                                         <button type="button" class="btn btn-default dropdown-toggle" data-toggle="dropdown" aria-haspopup="true"
                                                                 aria-expanded="false"><span class="caret"></span></button>
                                                         <ul class="dropdown-menu">
-                                                            <li><a href="{{ route('budget-limits.delete', [budgetLimit.id]) }}">{{ trans('firefly.remove_budgeted_amount', {currency: budgetLimit.currency_name }) }}</a></li>
+                                                            <li>
+                                                                <a class="delete_bl" href="#" data-budget-limit-id="{{ budgetLimit.id }}">{{ trans('firefly.remove_budgeted_amount', {currency: budgetLimit.currency_name }) }}</a>
+                                                            </li>
                                                         </ul>
                                                     </div>
                                                 </div>
                                                 <span class="text-danger budget_warning" data-id="{{ budget.id }}" data-budgetLimit="{{ budgetLimit.id }}"
                                                     style="display:none;"></span>
@@ -466,10 +468,11 @@
 
         // budget limit create form.
         var createBudgetLimitUri = "{{ route('budget-limits.create', ['REPLACEME', start.format('Y-m-d'), end.format('Y-m-d')]) }}";
         var storeBudgetLimitUri = "{{ route('budget-limits.store') }}";
         var updateBudgetLimitUri = "{{ route('budget-limits.update', ['REPLACEME']) }}";
+        var deleteBudgetLimitUrl = "{{ route('budget-limits.delete', ['REPLACEME']) }}";
         var totalBudgetedUri = "{{ route('json.budget.total-budgeted', ['REPLACEME', start.format('Y-m-d'), end.format('Y-m-d')]) }}";
 
         // period thing:
         var periodStart = "{{ start.format('Y-m-d') }}";
         var periodEnd = "{{ end.format('Y-m-d') }}";
diff --git a/routes/web.php b/routes/web.php
index 790cbc94ed..7982d04aaa 100644
--- a/routes/web.php
+++ b/routes/web.php
@@ -280,11 +280,11 @@ static function () {
     ['middleware' => 'user-full-auth', 'namespace' => 'FireflyIII\Http\Controllers', 'prefix' => 'budget-limits', 'as' => 'budget-limits.'],
     static function () {
         Route::get('create/{budget}/{start_date}/{end_date}', ['uses' => 'Budget\BudgetLimitController@create', 'as' => 'create']);
         Route::post('store', ['uses' => 'Budget\BudgetLimitController@store', 'as' => 'store']);
 
-        Route::get('delete/{budgetLimit}', ['uses' => 'Budget\BudgetLimitController@delete', 'as' => 'delete']);
+        Route::post('delete/{budgetLimit}', ['uses' => 'Budget\BudgetLimitController@delete', 'as' => 'delete']);
 
         Route::post('update/{budgetLimit}', ['uses' => 'Budget\BudgetLimitController@update', 'as' => 'update']);
     }
 );
 

Vulnerable code:     $.get(totalBudgetedUri.replace('REPLACEME',currencyId)).done(function (data) {
    var url = deleteBudgetLimitUrl.replace('REPLACEME', budgetId.toString();
                                                <div class="input-group">
                                                            <li><a href="{{ route('budget-limits.delete', [budgetLimit.id]) }}">{{ trans('firefly.remove_budgeted_amount', {currency: budgetLimit.currency_name }) }}</a></li>
        Route::get('delete/{budgetLimit}', ['uses' => 'Budget\BudgetLimitController@delete', 'as' => 'delete']);
Fixed code:     $.get(totalBudgetedUri.replace('REPLACEME', currencyId)).done(function (data) {
    e.preventDefault();
    var url = deleteBudgetLimitUrl.replace('REPLACEME', budgetLimitId.toString());
    $.post(url, {_token: token}).then(function () {
        $('.bl_entry[data-budget-limit-id="' + budgetLimitId + '"]').remove();
        
    });
                                                <div class="input-group bl_entry" data-budget-limit-id="{{ budgetLimit.id }}">
                                                            <li>
                                                                <a class="delete_bl" href="#" data-budget-limit-id="{{ budgetLimit.id }}">{{ trans('firefly.remove_budgeted_amount', {currency: budgetLimit.currency_name }) }}</a>
                                                            </li>
        var deleteBudgetLimitUrl = "{{ route('budget-limits.delete', ['REPLACEME']) }}";
        Route::post('delete/{budgetLimit}', ['uses' => 'Budget\BudgetLimitController@delete', 'as' => 'delete']);