Skip to content

Commit 02679b5

Browse files
nicolasalexandre9nicolasalexandre9
authored
fix: security vulnerabilities RCE on 8.x.x (#638)
1 parent 0633364 commit 02679b5

File tree

7 files changed

+199
-130
lines changed

7 files changed

+199
-130
lines changed

‎.github/workflows/build.yml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ on:
55
branches:
66
- main
77
- beta
8+
- 7.x.x
89
pull_request:
910

1011
env:
@@ -72,7 +73,7 @@ jobs:
7273
name: Release
7374
runs-on: ubuntu-latest
7475
needs: [lint, test]
75-
if: github.event_name == 'push' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/beta')
76+
if: github.event_name == 'push' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/beta' || github.ref == 'refs/heads/7.x.x')
7677
steps:
7778
- uses: actions/checkout@v2
7879
with:

‎.releaserc.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
module.exports = {
2-
branches: ['main', {name: 'beta', prerelease: true}],
2+
branches: ['main', '+([0-9])?(.{+([0-9]),x}).x', {name: 'beta', prerelease: true}],
33
plugins: [
44
[
55
'@semantic-release/commit-analyzer', {

‎app/services/forest_liana/stat_getter.rb‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,17 @@ def initialize(resource, params, forest_user)
66
@resource = resource
77
@params = params
88
@user = forest_user
9+
10+
validate_params
911
compute_includes
1012
end
1113

14+
def validate_params
15+
if @params.key?(:aggregator) && !%w[count sum avg max min].include?(@params[:aggregator].downcase)
16+
raise ForestLiana::Errors::HTTP422Error.new('Invalid aggregate function')
17+
end
18+
end
19+
1220
def get_resource
1321
super
1422
@resource.reorder('')

‎app/services/forest_liana/value_stat_getter.rb‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,24 +19,25 @@ def perform
1919
end
2020

2121
@record = Model::Stat.new(value: {
22-
countCurrent: count(resource),
23-
countPrevious: previous_value ? count(previous_value) : nil
22+
countCurrent: aggregate(resource),
23+
countPrevious: previous_value ? aggregate(previous_value) : nil
2424
})
2525
end
2626

2727
private
2828

29-
def count(value)
30-
uniq = @params[:aggregator].downcase == 'count'
29+
def aggregate(value)
30+
aggregator = @params[:aggregator].downcase
31+
uniq = aggregator == 'count'
3132

3233
if Rails::VERSION::MAJOR >= 4
3334
if uniq
3435
# NOTICE: uniq is deprecated since Rails 5.0
3536
value = Rails::VERSION::MAJOR >= 5 ? value.distinct : value.uniq
3637
end
37-
value.send(@params[:aggregator].downcase, aggregate_field)
38+
value.send(aggregator, aggregate_field)
3839
else
39-
value.send(@params[:aggregator].downcase, aggregate_field, distinct: uniq)
40+
value.send(aggregator, aggregate_field, distinct: uniq)
4041
end
4142
end
4243

‎spec/services/forest_liana/line_stat_getter_spec.rb‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,19 @@ module ForestLiana
99
allow(ForestLiana::ScopeManager).to receive(:fetch_scopes).and_return(scopes)
1010
end
1111

12+
describe 'with not allowed aggregator' do
13+
it 'should raise an error' do
14+
expect {
15+
LineStatGetter.new(Owner, {
16+
timezone: "Europe/Paris",
17+
aggregator: "eval",
18+
timeRange: "Week",
19+
groupByFieldName: "`ls`",
20+
}, user)
21+
}.to raise_error(ForestLiana::Errors::HTTP422Error, 'Invalid aggregate function')
22+
end
23+
end
24+
1225
describe 'Check client_timezone function' do
1326
describe 'with a SQLite database' do
1427
it 'should return false' do

‎spec/services/forest_liana/pie_stat_getter_spec.rb‎

Lines changed: 90 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -23,90 +23,113 @@ module ForestLiana
2323
}
2424
end
2525

26-
let(:model) { Tree }
27-
let(:collection) { 'trees' }
28-
let(:params) {
29-
{
30-
type: 'Pie',
31-
sourceCollectionName: collection,
32-
timezone: 'Europe/Paris',
33-
aggregator: 'Count',
34-
groupByFieldName: groupByFieldName
26+
describe 'with not allowed aggregator' do
27+
let(:scopes) { { } }
28+
let(:model) { Tree }
29+
let(:collection) { 'trees' }
30+
let(:params) {
31+
{
32+
type: 'Pie',
33+
sourceCollectionName: collection,
34+
timezone: 'Europe/Paris',
35+
aggregator: 'eval',
36+
groupByFieldName: '`ls`'
37+
}
3538
}
36-
}
3739

38-
subject { PieStatGetter.new(model, params, user) }
40+
it 'should raise an error' do
41+
expect {
42+
PieStatGetter.new(model, params, user)
43+
}.to raise_error(ForestLiana::Errors::HTTP422Error, 'Invalid aggregate function')
44+
end
45+
end
3946

40-
describe 'with empty scopes' do
41-
let(:scopes) { { } }
47+
describe 'with valid aggregate function' do
48+
let(:model) { Tree }
49+
let(:collection) { 'trees' }
50+
let(:params) {
51+
{
52+
type: 'Pie',
53+
sourceCollectionName: collection,
54+
timezone: 'Europe/Paris',
55+
aggregator: 'Count',
56+
groupByFieldName: groupByFieldName
57+
}
58+
}
4259

43-
describe 'with an aggregate on the name field' do
44-
let(:groupByFieldName) { 'name' }
45-
46-
it 'should be as many categories as records count' do
47-
subject.perform
48-
expect(subject.record.value).to match_array([
49-
{:key => "Old Tree n1", :value => 1},
50-
{:key => "Old Tree n2", :value => 1},
51-
{:key => "Old Tree n3", :value => 1},
52-
{:key => "Old Tree n4", :value => 1},
53-
{:key => "Young Tree n1", :value => 1},
54-
{:key => "Young Tree n2", :value => 1},
55-
{:key => "Young Tree n3", :value => 1},
56-
{:key => "Young Tree n4", :value => 1},
57-
{:key => "Young Tree n5", :value => 1}
58-
])
60+
subject { PieStatGetter.new(model, params, user) }
61+
62+
describe 'with empty scopes' do
63+
let(:scopes) { { } }
64+
65+
describe 'with an aggregate on the name field' do
66+
let(:groupByFieldName) { 'name' }
67+
68+
it 'should be as many categories as records count' do
69+
subject.perform
70+
expect(subject.record.value).to match_array([
71+
{:key => "Old Tree n1", :value => 1},
72+
{:key => "Old Tree n2", :value => 1},
73+
{:key => "Old Tree n3", :value => 1},
74+
{:key => "Old Tree n4", :value => 1},
75+
{:key => "Young Tree n1", :value => 1},
76+
{:key => "Young Tree n2", :value => 1},
77+
{:key => "Young Tree n3", :value => 1},
78+
{:key => "Young Tree n4", :value => 1},
79+
{:key => "Young Tree n5", :value => 1}
80+
])
81+
end
5982
end
60-
end
6183

62-
describe 'with an aggregate on the age field' do
63-
let(:groupByFieldName) { 'age' }
84+
describe 'with an aggregate on the age field' do
85+
let(:groupByFieldName) { 'age' }
6486

65-
it 'should be as many categories as different ages among records' do
66-
subject.perform
67-
expect(subject.record.value).to eq [{ :key => 3, :value => 5}, { :key => 15, :value => 4 }]
87+
it 'should be as many categories as different ages among records' do
88+
subject.perform
89+
expect(subject.record.value).to eq [{ :key => 3, :value => 5}, { :key => 15, :value => 4 }]
90+
end
6891
end
6992
end
70-
end
7193

72-
describe 'with scopes' do
73-
let(:scopes) {
74-
{
75-
'Tree' => {
76-
'scope'=> {
77-
'filter'=> {
78-
'aggregator' => 'and',
79-
'conditions' => [
80-
{ 'field' => 'age', 'operator' => 'less_than', 'value' => 10 }
81-
]
82-
},
83-
'dynamicScopesValues' => { }
94+
describe 'with scopes' do
95+
let(:scopes) {
96+
{
97+
'Tree' => {
98+
'scope'=> {
99+
'filter'=> {
100+
'aggregator' => 'and',
101+
'conditions' => [
102+
{ 'field' => 'age', 'operator' => 'less_than', 'value' => 10 }
103+
]
104+
},
105+
'dynamicScopesValues' => { }
106+
}
84107
}
85108
}
86109
}
87-
}
88110

89-
describe 'with an aggregate on the name field' do
90-
let(:groupByFieldName) { 'name' }
91-
92-
it 'should be as many categories as records inside the scope' do
93-
subject.perform
94-
expect(subject.record.value).to match_array([
95-
{:key => "Young Tree n1", :value => 1},
96-
{:key => "Young Tree n2", :value => 1},
97-
{:key => "Young Tree n3", :value => 1},
98-
{:key => "Young Tree n4", :value => 1},
99-
{:key => "Young Tree n5", :value => 1}
100-
])
111+
describe 'with an aggregate on the name field' do
112+
let(:groupByFieldName) { 'name' }
113+
114+
it 'should be as many categories as records inside the scope' do
115+
subject.perform
116+
expect(subject.record.value).to match_array([
117+
{:key => "Young Tree n1", :value => 1},
118+
{:key => "Young Tree n2", :value => 1},
119+
{:key => "Young Tree n3", :value => 1},
120+
{:key => "Young Tree n4", :value => 1},
121+
{:key => "Young Tree n5", :value => 1}
122+
])
123+
end
101124
end
102-
end
103125

104-
describe 'with an aggregate on the age field' do
105-
let(:groupByFieldName) { 'age' }
126+
describe 'with an aggregate on the age field' do
127+
let(:groupByFieldName) { 'age' }
106128

107-
it 'should be only one category' do
108-
subject.perform
109-
expect(subject.record.value).to eq [{ :key => 3, :value => 5}]
129+
it 'should be only one category' do
130+
subject.perform
131+
expect(subject.record.value).to eq [{ :key => 3, :value => 5}]
132+
end
110133
end
111134
end
112135
end

0 commit comments

Comments
 (0)