Add latest changes from gitlab-org/security/gitlab@13-11-stable-ee

5 files changed, 49 insertions, 5 deletions

diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb
index 87615d1b4f2..07e4a10708e 100644
--- a/app/services/issues/base_service.rb
+++ b/app/services/issues/base_service.rb
@@ -34,7 +34,7 @@ module Issues
 
     private
 
-    def filter_params(merge_request)
+    def filter_params(issue)
       super
 
       moved_issue = params.delete(:moved_issue)
@@ -44,6 +44,8 @@ module Issues
       params.delete(:iid) unless current_user.can?(:set_issue_iid, project)
       params.delete(:created_at) unless moved_issue || current_user.can?(:set_issue_created_at, project)
       params.delete(:updated_at) unless moved_issue || current_user.can?(:set_issue_updated_at, project)
+
+      issue.system_note_timestamp = params[:created_at] || params[:updated_at]
     end
 
     def create_assignee_note(issue, old_assignees)
diff --git a/changelogs/unreleased/security-disallow-changing-timestamps-on-issue-create-update.yml b/changelogs/unreleased/security-disallow-changing-timestamps-on-issue-create-update.yml
new file mode 100644
index 00000000000..7acd005abaa
--- /dev/null
+++ b/changelogs/unreleased/security-disallow-changing-timestamps-on-issue-create-update.yml
@@ -0,0 +1,5 @@
+---
+title: Restrict setting system_note_timestamp to owners
+merge_request:
+author:
+type: security
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 4f2ac73c0d3..c844655f0b3 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -249,7 +249,6 @@ module API
         authorize! :create_issue, user_project
 
         issue_params = declared_params(include_missing: false)
-        issue_params[:system_note_timestamp] = params[:created_at]
 
         issue_params = convert_parameters_from_legacy_format(issue_params)
 
@@ -293,8 +292,6 @@ module API
         issue = user_project.issues.find_by!(iid: params.delete(:issue_iid))
         authorize! :update_issue, issue
 
-        issue.system_note_timestamp = params[:updated_at]
-
         update_params = declared_params(include_missing: false).merge(request: request, api: true)
 
         update_params = convert_parameters_from_legacy_format(update_params)
diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb
index 0fe68be027c..8f10de59526 100644
--- a/spec/requests/api/issues/issues_spec.rb
+++ b/spec/requests/api/issues/issues_spec.rb
@@ -943,6 +943,34 @@ RSpec.describe API::Issues do
     it_behaves_like 'issuable update endpoint' do
       let(:entity) { issue }
     end
+
+    describe 'updated_at param' do
+      let(:fixed_time) { Time.new(2001, 1, 1) }
+      let(:updated_at) { Time.new(2000, 1, 1) }
+
+      before do
+        travel_to fixed_time
+      end
+
+      it 'allows admins to set the timestamp' do
+        put api("/projects/#{project.id}/issues/#{issue.iid}", admin), params: { labels: 'label1', updated_at: updated_at }
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(Time.parse(json_response['updated_at'])).to be_like_time(updated_at)
+        expect(ResourceLabelEvent.last.created_at).to be_like_time(updated_at)
+      end
+
+      it 'does not allow other users to set the timestamp' do
+        reporter = create(:user)
+        project.add_developer(reporter)
+
+        put api("/projects/#{project.id}/issues/#{issue.iid}", reporter), params: { labels: 'label1', updated_at: updated_at }
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(Time.parse(json_response['updated_at'])).to be_like_time(fixed_time)
+        expect(ResourceLabelEvent.last.created_at).to be_like_time(fixed_time)
+      end
+    end
   end
 
   describe 'DELETE /projects/:id/issues/:issue_iid' do
diff --git a/spec/requests/api/issues/post_projects_issues_spec.rb b/spec/requests/api/issues/post_projects_issues_spec.rb
index 7f1db620d4f..9d3bd26a200 100644
--- a/spec/requests/api/issues/post_projects_issues_spec.rb
+++ b/spec/requests/api/issues/post_projects_issues_spec.rb
@@ -330,15 +330,21 @@ RSpec.describe API::Issues do
     end
 
     context 'setting created_at' do
+      let(:fixed_time) { Time.new(2001, 1, 1) }
       let(:creation_time) { 2.weeks.ago }
       let(:params) { { title: 'new issue', labels: 'label, label2', created_at: creation_time } }
 
+      before do
+        travel_to fixed_time
+      end
+
       context 'by an admin' do
         it 'sets the creation time on the new issue' do
           post api("/projects/#{project.id}/issues", admin), params: params
 
           expect(response).to have_gitlab_http_status(:created)
           expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
+          expect(ResourceLabelEvent.last.created_at).to be_like_time(creation_time)
         end
       end
 
@@ -348,6 +354,7 @@ RSpec.describe API::Issues do
 
           expect(response).to have_gitlab_http_status(:created)
           expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
+          expect(ResourceLabelEvent.last.created_at).to be_like_time(creation_time)
         end
       end
 
@@ -356,19 +363,24 @@ RSpec.describe API::Issues do
           group = create(:group)
           group_project = create(:project, :public, namespace: group)
           group.add_owner(user2)
+
           post api("/projects/#{group_project.id}/issues", user2), params: params
 
           expect(response).to have_gitlab_http_status(:created)
           expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
+          expect(ResourceLabelEvent.last.created_at).to be_like_time(creation_time)
         end
       end
 
       context 'by another user' do
         it 'ignores the given creation time' do
+          project.add_developer(user2)
+
           post api("/projects/#{project.id}/issues", user2), params: params
 
           expect(response).to have_gitlab_http_status(:created)
-          expect(Time.parse(json_response['created_at'])).not_to be_like_time(creation_time)
+          expect(Time.parse(json_response['created_at'])).to be_like_time(fixed_time)
+          expect(ResourceLabelEvent.last.created_at).to be_like_time(fixed_time)
         end
       end
     end