Wrap call to rest2html in a timeout
Description
Wraps command invocations in a timeout
, which will kill the command after 10 seconds.
The timeout seconds can be controlled via the GITLAB_MARKUP_TIMEOUT
environment variable.
If the command times out, the process will be killed with a KILL
signal, and a TimeoutError
with a descriptive message will be raised.
If you set the timeout greater than 60 seconds and load one that takes longer (e.g. dos
original test file with GITLAB_MARKUP_TIMEOUT
set to 120 seconds), then:
- Client will get
Gitlab::RequestContext::RequestDeadlineExceeded at /root/rst_dos_test/-/wikis/dos
after ~60 seconds - The worker is still killed after the timeout of 120 seconds even though the request has timed out (thus fixing the original exploit from https://gitlab.com/gitlab-org/gitlab/-/issues/37283).
See https://gitlab.com/gitlab-org/gitlab/-/issues/37283#note_725442087 for more context.
Screenshots
Successful render (no timeout):
Process killed by timeout (no render)
log/exceptions_json.log
Example of {"severity":"ERROR","time":"2021-11-29T18:47:57.170Z","correlation_id":"01FNPG8MGAGEP3JXW6HDJ4KBS5","exception.class":"Timeout::Error","exception.message":"Command was killed, probably due to exceeding GITLAB_MARKUP_TIMEOUT limit of 120 seconds","exception.backtrace":["lib/gitlab/other_markup.rb:11:in `render'","app/helpers/markup_helper.rb:281:in `other_markup_unsafe'","app/helpers/markup_helper.rb:146:in `markup_unsafe'","app/helpers/markup_helper.rb:131:in `render_wiki_content'","app/views/shared/wikis/show.html.haml:30","app/controllers/application_controller.rb:136:in `render'","app/controllers/concerns/wiki_actions.rb:84:in `show'","ee/lib/gitlab/ip_address_state.rb:10:in `with'","ee/app/controllers/ee/application_controller.rb:44:in `set_current_ip_address'","app/controllers/application_controller.rb:504:in `set_current_admin'","lib/gitlab/session.rb:11:in `with_session'","app/controllers/application_controller.rb:495:in `set_session_storage'","lib/gitlab/i18n.rb:105:in `with_locale'","lib/gitlab/i18n.rb:111:in `with_user_locale'","app/controllers/application_controller.rb:489:in `set_locale'","app/controllers/application_controller.rb:483:in `set_current_context'","lib/gitlab/metrics/elasticsearch_rack_middleware.rb:16:in `call'","lib/gitlab/middleware/rails_queue_duration.rb:33:in `call'","lib/gitlab/middleware/speedscope.rb:13:in `call'","lib/gitlab/request_profiler/middleware.rb:17:in `call'","lib/gitlab/query_limiting/middleware.rb:17:in `block in call'","lib/gitlab/query_limiting/transaction.rb:40:in `run'","lib/gitlab/query_limiting/middleware.rb:16:in `call'","lib/gitlab/metrics/rack_middleware.rb:16:in `block in call'","lib/gitlab/metrics/web_transaction.rb:46:in `run'","lib/gitlab/metrics/rack_middleware.rb:16:in `call'","lib/gitlab/jira/middleware.rb:19:in `call'","lib/gitlab/middleware/go.rb:20:in `call'","lib/gitlab/etag_caching/middleware.rb:21:in `call'","lib/gitlab/middleware/query_analyzer.rb:11:in `block in call'","lib/gitlab/database/query_analyzer.rb:42:in `within'","lib/gitlab/middleware/query_analyzer.rb:11:in `call'","lib/gitlab/middleware/multipart.rb:173:in `call'","lib/gitlab/middleware/read_only/controller.rb:50:in `call'","lib/gitlab/middleware/read_only.rb:18:in `call'","lib/gitlab/middleware/same_site_cookies.rb:27:in `call'","lib/gitlab/middleware/handle_malformed_strings.rb:21:in `call'","lib/gitlab/middleware/basic_health_check.rb:25:in `call'","lib/gitlab/middleware/handle_ip_spoof_attack_error.rb:25:in `call'","lib/gitlab/middleware/request_context.rb:21:in `call'","config/initializers/fix_local_cache_middleware.rb:11:in `call'","lib/gitlab/middleware/compressed_json.rb:26:in `call'","lib/gitlab/middleware/static.rb:11:in `call'","lib/gitlab/webpack/dev_server_middleware.rb:34:in `perform_request'","lib/gitlab/middleware/rack_multipart_tempfile_factory.rb:19:in `call'","lib/gitlab/middleware/sidekiq_web_static.rb:20:in `call'","lib/gitlab/metrics/requests_rack_middleware.rb:75:in `call'","lib/gitlab/middleware/release_env.rb:13:in `call'"],"user.username":"root","tags.program":"web","tags.locale":"en","tags.feature_category":"wiki","tags.correlation_id":"01FNPG8MGAGEP3JXW6HDJ4KBS5","extra.project_id":20,"extra.file_name":"dos.rst"}
Testing
Steps to reproduce
See the reproduce
section in the issue for steps to reproduce: https://gitlab.com/gitlab-org/gitlab/-/issues/37283#reproduce
Additional testing notes
Ensure you have docutils
installed:
python3 -m pip install --upgrade setuptools
python3 -m pip install --upgrade pip
pip3 install docutils
I have verified that the timeout
CLI options are the same on MacOS and Linux, and @cmiskell confirmed that this should be supported in all recent OSs
Test locally by replacing the following line to your GitLab Gemfile, bundle
and gdk restart rails-web
:
gem 'gitlab-markup', '~> 1.7.1', path: '/your/path/to/gitlab-markup/'
See the following repo for examples of large RST files: https://gitlab.com/gitlab-com/gl-security/security-research/sec-research/-/tree/master/005/rst
Here is also an example of a small, valid RST file: example.rst
If you see the following .. math::
as the first line, it means the RST did NOT render due to the timeout.
.. math::
\Huge \sqrt{\sqrt{\sqrt{\sqrt{
If you don't see it, then it rendered successfully. You can also add some fixed text in triple-backticks to easily confirm whether it rendered or not.
You can also tail -f log/exceptions_json.log
to look for the errors.
This can also be verified in the debugger by setting a breakpoint in gitlab-markup/lib/github/markup/command_implementation.rb
and checking the status of the executed command.
To test the posix branch, add gem "posix-spawn", :platforms => :ruby
to your gemfile and restart.
My my machine, currently the sample dos
file sized just under 1 meg renders successfully, but the ones 2 meg and above time out.
Gemfile Patch
Here's a patch for the Gemfile/Gemfile.lock changes:
Index: Gemfile.lock
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Gemfile.lock b/Gemfile.lock
--- a/Gemfile.lock (revision 7e9466e331e17476bac781ed4447cce86d276ebf)
+++ b/Gemfile.lock (date 1638207874727)
@@ -1,3 +1,8 @@
+PATH
+ remote: /Users/cwoolley/workspace/gitlab-markup
+ specs:
+ gitlab-markup (1.7.1)
+
PATH
remote: vendor/gems/mail-smtp_pool
specs:
@@ -489,7 +494,6 @@
with_env (= 1.1.0)
xml-simple (~> 1.1.5)
gitlab-mail_room (0.0.9)
- gitlab-markup (1.7.1)
gitlab-net-dns (0.9.1)
gitlab-omniauth-openid-connect (0.8.0)
addressable (~> 2.7)
@@ -918,6 +922,7 @@
png_quantizator (0.2.1)
po_to_json (1.0.1)
json (>= 1.6.0)
+ posix-spawn (0.3.15)
premailer (1.11.1)
addressable
css_parser (>= 1.6.0)
@@ -1475,7 +1480,7 @@
gitlab-license (~> 2.0)
gitlab-license_finder (~> 6.0)
gitlab-mail_room (~> 0.0.9)
- gitlab-markup (~> 1.7.1)
+ gitlab-markup (~> 1.7.1)!
gitlab-net-dns (~> 0.9.1)
gitlab-omniauth-openid-connect (~> 0.8.0)
gitlab-sidekiq-fetcher (= 0.8.0)
@@ -1569,6 +1574,7 @@
pg (~> 1.1)
pg_query (~> 2.1)
png_quantizator (~> 0.2.1)
+ posix-spawn
premailer-rails (~> 1.10.3)
prometheus-client-mmap (~> 0.15.0)
pry-byebug
Index: Gemfile
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Gemfile b/Gemfile
--- a/Gemfile (revision 7e9466e331e17476bac781ed4447cce86d276ebf)
+++ b/Gemfile (date 1638207880974)
@@ -153,7 +153,9 @@
# Markdown and HTML processing
gem 'html-pipeline', '~> 2.13.2'
gem 'deckar01-task_list', '2.3.1'
-gem 'gitlab-markup', '~> 1.7.1'
+# gem 'gitlab-markup', '~> 1.7.1'
+gem 'gitlab-markup', '~> 1.7.1', path: '/Users/cwoolley/workspace/gitlab-markup/'
+gem "posix-spawn", :platforms => :ruby
gem 'github-markup', '~> 1.7.0', require: 'github/markup'
gem 'commonmarker', '~> 0.23.2'
gem 'kramdown', '~> 2.3.1'