fix: security vulnerabilities RCE (#637)

Author: nicolasalexandre9

.github/workflows/build.yml

@@ -5,6 +5,7 @@ on:
     branches:
       - main
       - beta
+      - 7.x.x
   pull_request:
 
 env:

.releaserc.js

@@ -1,5 +1,5 @@
 module.exports = {
-  branches: ['main', {name: 'beta', prerelease: true}],
+  branches: ['main', '+([0-9])?(.{+([0-9]),x}).x', {name: 'beta', prerelease: true}],
   plugins: [
     [
       '@semantic-release/commit-analyzer', {

app/services/forest_liana/stat_getter.rb

@@ -6,7 +6,15 @@ def initialize(resource, params, forest_user)
       @resource = resource
       @params = params
       @user = forest_user
-      compute_includes()
+
+      validate_params
+      compute_includes
+    end
+
+    def validate_params
+      if @params.key?(:aggregate) && !%w[count sum avg max min].include?(@params[:aggregate].downcase)
+        raise ForestLiana::Errors::HTTP422Error.new('Invalid aggregate function')
+      end
     end
 
     def get_resource

app/services/forest_liana/value_stat_getter.rb

@@ -19,24 +19,25 @@ def perform
       end
 
       @record = Model::Stat.new(value: {
-        countCurrent: count(resource),
-        countPrevious: previous_value ? count(previous_value) : nil
+        countCurrent: aggregate(resource),
+        countPrevious: previous_value ? aggregate(previous_value) : nil
       })
     end
 
     private
 
-    def count(value)
-      uniq = @params[:aggregate].downcase == 'count'
+    def aggregate(value)
+      aggregator = @params[:aggregate].downcase
+      uniq = aggregator == 'count'
 
       if Rails::VERSION::MAJOR >= 4
         if uniq
           # NOTICE: uniq is deprecated since Rails 5.0
           value = Rails::VERSION::MAJOR >= 5 ? value.distinct : value.uniq
         end
-        value.send(@params[:aggregate].downcase, aggregate_field)
+        value.send(aggregator, aggregate_field)
       else
-        value.send(@params[:aggregate].downcase, aggregate_field, distinct: uniq)
+        value.send(aggregator, aggregate_field, distinct: uniq)
       end
     end
 

spec/services/forest_liana/line_stat_getter_spec.rb

@@ -10,6 +10,19 @@ module ForestLiana
       Owner.delete_all
     end
 
+    describe 'with not allowed aggregator' do
+      it 'should raise an error' do
+        expect {
+          LineStatGetter.new(Owner, {
+            timezone: "Europe/Paris",
+            aggregate: "eval",
+            time_range: "Week",
+            group_by_date_field: "`ls`",
+          }, user)
+        }.to raise_error(ForestLiana::Errors::HTTP422Error, 'Invalid aggregate function')
+      end
+    end
+
     describe 'Check client_timezone function' do
       describe 'with a SQLite database' do
         it 'should return false' do

spec/services/forest_liana/pie_stat_getter_spec.rb

@@ -23,90 +23,113 @@ module ForestLiana
       }
     end
 
-    let(:model) { Tree }
-    let(:collection) { 'trees' }
-    let(:params) {
-      {
-        type: 'Pie',
-        collection: collection,
-        timezone: 'Europe/Paris',
-        aggregate: 'Count',
-        group_by_field: group_by_field
+    describe 'with not allowed aggregator' do
+      let(:scopes) { { } }
+      let(:model) { Tree }
+      let(:collection) { 'trees' }
+      let(:params) {
+        {
+          type: 'Pie',
+          collection: collection,
+          timezone: 'Europe/Paris',
+          aggregate: 'eval',
+          group_by_field: '`ls`'
+        }
       }
-    }
 
-    subject { PieStatGetter.new(model, params, user) }
+      it 'should raise an error' do
+        expect {
+          PieStatGetter.new(model, params, user)
+        }.to raise_error(ForestLiana::Errors::HTTP422Error, 'Invalid aggregate function')
+      end
+    end
 
-    describe 'with empty scopes' do
-      let(:scopes) { { } }
+    describe 'with valid aggregate function' do
+      let(:model) { Tree }
+      let(:collection) { 'trees' }
+      let(:params) {
+        {
+          type: 'Pie',
+          collection: collection,
+          timezone: 'Europe/Paris',
+          aggregate: 'Count',
+          group_by_field: group_by_field
+        }
+      }
 
-      describe 'with an aggregate on the name field' do
-        let(:group_by_field) { 'name' }
-
-        it 'should be as many categories as records count' do
-          subject.perform
-          expect(subject.record.value).to match_array([
-            {:key => "Old Tree n1", :value => 1},
-            {:key => "Old Tree n2", :value => 1},
-            {:key => "Old Tree n3", :value => 1},
-            {:key => "Old Tree n4", :value => 1},
-            {:key => "Young Tree n1", :value => 1},
-            {:key => "Young Tree n2", :value => 1},
-            {:key => "Young Tree n3", :value => 1},
-            {:key => "Young Tree n4", :value => 1},
-            {:key => "Young Tree n5", :value => 1}
-          ])
+      subject { PieStatGetter.new(model, params, user) }
+
+      describe 'with empty scopes' do
+        let(:scopes) { { } }
+
+        describe 'with an aggregate on the name field' do
+          let(:group_by_field) { 'name' }
+
+          it 'should be as many categories as records count' do
+            subject.perform
+            expect(subject.record.value).to match_array([
+              {:key => "Old Tree n1", :value => 1},
+              {:key => "Old Tree n2", :value => 1},
+              {:key => "Old Tree n3", :value => 1},
+              {:key => "Old Tree n4", :value => 1},
+              {:key => "Young Tree n1", :value => 1},
+              {:key => "Young Tree n2", :value => 1},
+              {:key => "Young Tree n3", :value => 1},
+              {:key => "Young Tree n4", :value => 1},
+              {:key => "Young Tree n5", :value => 1}
+            ])
+          end
         end
-      end
 
-      describe 'with an aggregate on the age field' do
-        let(:group_by_field) { 'age' }
+        describe 'with an aggregate on the age field' do
+          let(:group_by_field) { 'age' }
 
-        it 'should be as many categories as different ages among records' do
-          subject.perform
-          expect(subject.record.value).to eq [{ :key => 3, :value => 5}, { :key => 15, :value => 4 }]
+          it 'should be as many categories as different ages among records' do
+            subject.perform
+            expect(subject.record.value).to eq [{ :key => 3, :value => 5}, { :key => 15, :value => 4 }]
+          end
         end
       end
-    end
 
-    describe 'with scopes' do
-      let(:scopes) {
-        {
-          'Tree' => {
-            'scope'=> {
-              'filter'=> {
-                'aggregator' => 'and',
-                'conditions' => [
-                  { 'field' => 'age', 'operator' => 'less_than', 'value' => 10 }
-                ]
-              },
-              'dynamicScopesValues' => { }
+      describe 'with scopes' do
+        let(:scopes) {
+          {
+            'Tree' => {
+              'scope'=> {
+                'filter'=> {
+                  'aggregator' => 'and',
+                  'conditions' => [
+                    { 'field' => 'age', 'operator' => 'less_than', 'value' => 10 }
+                  ]
+                },
+                'dynamicScopesValues' => { }
+              }
             }
           }
         }
-      }
 
-      describe 'with an aggregate on the name field' do
-        let(:group_by_field) { 'name' }
-
-        it 'should be as many categories as records inside the scope' do
-          subject.perform
-          expect(subject.record.value).to match_array([
-            {:key => "Young Tree n1", :value => 1},
-            {:key => "Young Tree n2", :value => 1},
-            {:key => "Young Tree n3", :value => 1},
-            {:key => "Young Tree n4", :value => 1},
-            {:key => "Young Tree n5", :value => 1}
-          ])
+        describe 'with an aggregate on the name field' do
+          let(:group_by_field) { 'name' }
+
+          it 'should be as many categories as records inside the scope' do
+            subject.perform
+            expect(subject.record.value).to match_array([
+              {:key => "Young Tree n1", :value => 1},
+              {:key => "Young Tree n2", :value => 1},
+              {:key => "Young Tree n3", :value => 1},
+              {:key => "Young Tree n4", :value => 1},
+              {:key => "Young Tree n5", :value => 1}
+            ])
+          end
         end
-      end
 
-      describe 'with an aggregate on the age field' do
-        let(:group_by_field) { 'age' }
+        describe 'with an aggregate on the age field' do
+          let(:group_by_field) { 'age' }
 
-        it 'should be only one category' do
-          subject.perform
-          expect(subject.record.value).to eq [{ :key => 3, :value => 5}]
+          it 'should be only one category' do
+            subject.perform
+            expect(subject.record.value).to eq [{ :key => 3, :value => 5}]
+          end
         end
       end
     end