From c22276f74f9de7fa8fa69d42db2e32b1455b6b1d Mon Sep 17 00:00:00 2001 From: f Date: Thu, 11 Jan 2024 18:13:52 -0300 Subject: [PATCH 1/3] =?UTF-8?q?fix:=20especificar=20qu=C3=A9=20parametros?= =?UTF-8?q?=20de=20airbrake=20permitimos=20#14956?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/controllers/api/v1/notices_controller.rb | 38 ++++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/v1/notices_controller.rb b/app/controllers/api/v1/notices_controller.rb index 3d74a48f..8f384f1a 100644 --- a/app/controllers/api/v1/notices_controller.rb +++ b/app/controllers/api/v1/notices_controller.rb @@ -9,10 +9,10 @@ module Api # Generar un stacktrace en segundo plano y enviarlo por correo # solo si la API key es verificable. Del otro lado siempre # respondemos con lo mismo. - def create + def create if (site&.airbrake_valid? airbrake_token) && !detected_device.bot? BacktraceJob.perform_later site_id: params[:site_id], - params: airbrake_params.to_h + params: airbrake_params.to_h end render status: 201, json: { id: 1, url: '' } @@ -23,7 +23,39 @@ module Api # XXX: Por alguna razón Airbrake envía los datos con Content-Type: # text/plain. def airbrake_params - @airbrake_params ||= params.merge!(FastJsonparser.parse(request.raw_post) || {}).permit! + @airbrake_params ||= + params.merge!(FastJsonparser.parse(request.raw_post) || {}) + .permit( + { + errors: [ + :type, + :message, + { backtrace: %i[file line column function] } + ] + }, + { + context: [ + :url, + :language, + :severity, + :userAgent, + :windowError, + :rootDirectory, + { + history: [ + :date, + :type, + :severity, + :target, + :method, + :duration, + :statusCode, + { arguments: [] } + ] + } + ] + } + ) end def site From 22bd58054a8f024c11344211c4506386bd7e1ec4 Mon Sep 17 00:00:00 2001 From: f Date: Thu, 11 Jan 2024 18:14:21 -0300 Subject: [PATCH 2/3] fix: deprecar carga de archivo desde el sitio #14956 --- app/controllers/sites_controller.rb | 21 --------------------- app/models/post.rb | 4 +++- config/routes.rb | 3 --- 3 files changed, 3 insertions(+), 25 deletions(-) diff --git a/app/controllers/sites_controller.rb b/app/controllers/sites_controller.rb index 17287eb0..bec42b39 100644 --- a/app/controllers/sites_controller.rb +++ b/app/controllers/sites_controller.rb @@ -110,27 +110,6 @@ class SitesController < ApplicationController redirect_to sites_path end - # Obtiene y streamea archivos estáticos desde el repositorio mismo, - # pero sólo los públicos (es decir los archivos subidos desde Sutty). - def static_file - authorize site - - file = params.require(:file) + '.' + params.require(:format) - - raise ActionController::RoutingError.new(nil, nil) unless file.start_with? 'public/' - - path = site.relative_path file - - raise ActionController::RoutingError.new(nil, nil) unless File.exist? path - - # TODO: Hacer esto usa recursos, pero menos que generar el sitio - # cada vez. Para poder usar X-Accel tendríamos que montar los - # repositorios en el servidor web, cosa que no queremos, o hacer - # links simbólicos desde todos los public, o usar un servidor web - # local que soporte sendfile mejor que Rails (nghttpd?) - send_file path - end - private def site diff --git a/app/models/post.rb b/app/models/post.rb index 9aa0ac61..8885897f 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -103,8 +103,10 @@ class Post src = element.attributes['src'] next unless src&.value&.start_with? 'public/' + file = MetadataFile.new(site: site, post: self, document: document, layout: layout) + file.value['path'] = src.value - src.value = Rails.application.routes.url_helpers.site_static_file_url(site, file: src.value) + src.value = Rails.application.routes.url_helpers.url_for(file.static_file) end # Notificar a les usuaries que están viendo una previsualización diff --git a/config/routes.rb b/config/routes.rb index f2487066..635be07a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -28,9 +28,6 @@ Rails.application.routes.draw do # alias en nginx sin tener que usar expresiones regulares para # detectar el nombre del sitio. get '/sites/private/:site_id(*file)', to: 'private#show', constraints: { site_id: %r{[^/]+} } - # Obtener archivos estáticos desde el directorio público - get '/sites/:site_id/static_file/(*file)', to: 'sites#static_file', as: 'site_static_file', - constraints: { site_id: %r{[^/]+} } get '/env.js', to: 'env#index' match '/api/v3/projects/:site_id/notices' => 'api/v1/notices#create', via: %i[post] From 1bed78345c6a478724fbb6a6525053fc62551d28 Mon Sep 17 00:00:00 2001 From: f Date: Thu, 11 Jan 2024 18:16:38 -0300 Subject: [PATCH 3/3] fix: ignorar alertas de brakeman MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * las redirecciones son necesarias para poder reenviar a las páginas de agradecimiento y no podemos saber la url de antemano. * Site.domain no es un atributo sino una configuración estática * Site#tienda_url es un atributo estático * no sabemos todos los parametros de antemano en los breadcrumbs --- config/brakeman.ignore | 252 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 252 insertions(+) create mode 100644 config/brakeman.ignore diff --git a/config/brakeman.ignore b/config/brakeman.ignore new file mode 100644 index 00000000..137d2090 --- /dev/null +++ b/config/brakeman.ignore @@ -0,0 +1,252 @@ +{ + "ignored_warnings": [ + { + "warning_type": "Redirect", + "warning_code": 18, + "fingerprint": "0ae5c3990d49dfbfd4fd61874451f7a576d5056aca913068adf58c314625f810", + "check_name": "Redirect", + "message": "Possible unprotected redirect", + "file": "app/controllers/api/v1/posts_controller.rb", + "line": 20, + "link": "https://brakemanscanner.org/docs/warning_types/redirect/", + "code": "redirect_to((params[:redirect_to] or origin.to_s))", + "render_path": null, + "location": { + "type": "method", + "class": "Api::V1::PostsController", + "method": "create" + }, + "user_input": "params[:redirect_to]", + "confidence": "High", + "cwe_id": [ + 601 + ], + "note": "" + }, + { + "warning_type": "Denial of Service", + "warning_code": 76, + "fingerprint": "1947d1a2ae6e4bf718d0cc563e660efca96897165e9a8dd18186c1d7abe6ddf6", + "check_name": "RegexDoS", + "message": "Model attribute used in regular expression", + "file": "app/controllers/api/v1/base_controller.rb", + "line": 20, + "link": "https://brakemanscanner.org/docs/warning_types/denial_of_service/", + "code": "/\\.#{Site.domain}\\z/", + "render_path": null, + "location": { + "type": "method", + "class": "Api::V1::BaseController", + "method": "site_id" + }, + "user_input": "Site.domain", + "confidence": "Medium", + "cwe_id": [ + 20, + 185 + ], + "note": "No es un atributo, es una variable de entorno" + }, + { + "warning_type": "Cross-Site Scripting", + "warning_code": 4, + "fingerprint": "28d98d08a15c4b3ad94a2cfa20a12573de12d99f1a30b3ca51074ee1f1886592", + "check_name": "LinkToHref", + "message": "Potentially unsafe model attribute in `link_to` href", + "file": "app/views/layouts/_breadcrumb.haml", + "line": 19, + "link": "https://brakemanscanner.org/docs/warning_types/link_to_href", + "code": "link_to(t(\".tienda\"), Site.find(params[:site_id]).tienda_url, :role => \"button\", :class => \"btn\")", + "render_path": [ + { + "type": "controller", + "class": "Api::V1::NoticesController", + "method": "site", + "line": 31, + "file": "app/controllers/api/v1/notices_controller.rb", + "rendered": { + "name": "layouts/application", + "file": "app/views/layouts/application.html.haml" + } + }, + { + "type": "template", + "name": "layouts/application", + "line": 25, + "file": "app/views/layouts/application.html.haml", + "rendered": { + "name": "layouts/_breadcrumb", + "file": "app/views/layouts/_breadcrumb.haml" + } + } + ], + "location": { + "type": "template", + "template": "layouts/_breadcrumb" + }, + "user_input": "Site.find(params[:site_id]).tienda_url", + "confidence": "Weak", + "cwe_id": [ + 79 + ], + "note": "" + }, + { + "warning_type": "Redirect", + "warning_code": 18, + "fingerprint": "5034e51aaa1bac06d15fdde5956edffbfd65f94f5620a409526bbea896dc7b5f", + "check_name": "Redirect", + "message": "Possible unprotected redirect", + "file": "app/controllers/api/v1/contact_controller.rb", + "line": 26, + "link": "https://brakemanscanner.org/docs/warning_types/redirect/", + "code": "redirect_to((params[:redirect] or origin.to_s))", + "render_path": null, + "location": { + "type": "method", + "class": "Api::V1::ContactController", + "method": "receive" + }, + "user_input": "params[:redirect]", + "confidence": "High", + "cwe_id": [ + 601 + ], + "note": "" + }, + { + "warning_type": "Mass Assignment", + "warning_code": 70, + "fingerprint": "50582f39f8dfa900d3f2b5b9908b1592f8b8bd9e2d0b9d1cc05d77e5ede2d94e", + "check_name": "MassAssignment", + "message": "Specify exact keys allowed for mass assignment instead of using `permit!` which allows any keys", + "file": "app/views/layouts/_link_rel_alternate.haml", + "line": 2, + "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", + "code": "params.permit!", + "render_path": [ + { + "type": "controller", + "class": "Api::V1::BaseController", + "method": "site_id", + "line": 20, + "file": "app/controllers/api/v1/base_controller.rb", + "rendered": { + "name": "layouts/application", + "file": "app/views/layouts/application.html.haml" + } + }, + { + "type": "template", + "name": "layouts/application", + "line": 21, + "file": "app/views/layouts/application.html.haml", + "rendered": { + "name": "layouts/_link_rel_alternate", + "file": "app/views/layouts/_link_rel_alternate.haml" + } + } + ], + "location": { + "type": "template", + "template": "layouts/_link_rel_alternate" + }, + "user_input": null, + "confidence": "Medium", + "cwe_id": [ + 915 + ], + "note": "" + }, + { + "warning_type": "Mass Assignment", + "warning_code": 70, + "fingerprint": "b8e0aa898288bebb614ccc1340d169caa196d315c6ac2e4744081cc892c2ae97", + "check_name": "MassAssignment", + "message": "Specify exact keys allowed for mass assignment instead of using `permit!` which allows any keys", + "file": "app/views/layouts/_breadcrumb.haml", + "line": 30, + "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", + "code": "params.permit!", + "render_path": [ + { + "type": "controller", + "class": "Api::V1::BaseController", + "method": "site_id", + "line": 20, + "file": "app/controllers/api/v1/base_controller.rb", + "rendered": { + "name": "layouts/application", + "file": "app/views/layouts/application.html.haml" + } + }, + { + "type": "template", + "name": "layouts/application", + "line": 25, + "file": "app/views/layouts/application.html.haml", + "rendered": { + "name": "layouts/_breadcrumb", + "file": "app/views/layouts/_breadcrumb.haml" + } + } + ], + "location": { + "type": "template", + "template": "layouts/_breadcrumb" + }, + "user_input": null, + "confidence": "Medium", + "cwe_id": [ + 915 + ], + "note": "" + }, + { + "warning_type": "Cross-Site Scripting", + "warning_code": 4, + "fingerprint": "c051421c7cf4c2706b8e27bfd2f3b0661ec6a6df873da322a6b634b59e80351b", + "check_name": "LinkToHref", + "message": "Potentially unsafe model attribute in `link_to` href", + "file": "app/views/sites/_form.haml", + "line": 74, + "link": "https://brakemanscanner.org/docs/warning_types/link_to_href", + "code": "link_to(t(\".design.url\"), (Unresolved Model).new.url, :target => \"_blank\", :class => \"btn\")", + "render_path": [ + { + "type": "controller", + "class": "SitesController", + "method": "new", + "line": 31, + "file": "app/controllers/sites_controller.rb", + "rendered": { + "name": "sites/new", + "file": "app/views/sites/new.haml" + } + }, + { + "type": "template", + "name": "sites/new", + "line": 6, + "file": "app/views/sites/new.haml", + "rendered": { + "name": "sites/_form", + "file": "app/views/sites/_form.haml" + } + } + ], + "location": { + "type": "template", + "template": "sites/_form" + }, + "user_input": "(Unresolved Model).new.url", + "confidence": "Weak", + "cwe_id": [ + 79 + ], + "note": "" + } + ], + "updated": "2024-01-11 18:12:14 -0300", + "brakeman_version": "5.4.1" +}