Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport security fixes from upstream #494

Merged
merged 8 commits into from
Jul 6, 2023
37 changes: 37 additions & 0 deletions app/lib/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,48 @@
# Monkey-patch the HTTP.rb timeout class to avoid using a timeout block
# around the Socket#open method, since we use our own timeout blocks inside
# that method
#
# Also changes how the read timeout behaves so that it is cumulative (closer
# to HTTP::Timeout::Global, but still having distinct timeouts for other
# operation types)
class HTTP::Timeout::PerOperation
def connect(socket_class, host, port, nodelay = false)
@socket = socket_class.open(host, port)
@socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay
end

# Reset deadline when the connection is re-used for different requests
def reset_counter
@deadline = nil
end

# Read data from the socket
def readpartial(size, buffer = nil)
@deadline ||= Process.clock_gettime(Process::CLOCK_MONOTONIC) + @read_timeout

timeout = false
loop do
result = @socket.read_nonblock(size, buffer, exception: false)

return :eof if result.nil?

remaining_time = @deadline - Process.clock_gettime(Process::CLOCK_MONOTONIC)
raise HTTP::TimeoutError, "Read timed out after #{@read_timeout} seconds" if timeout || remaining_time <= 0
return result if result != :wait_readable

# marking the socket for timeout. Why is this not being raised immediately?
# it seems there is some race-condition on the network level between calling
# #read_nonblock and #wait_readable, in which #read_nonblock signalizes waiting
# for reads, and when waiting for x seconds, it returns nil suddenly without completing
# the x seconds. In a normal case this would be a timeout on wait/read, but it can
# also mean that the socket has been closed by the server. Therefore we "mark" the
# socket for timeout and try to read more bytes. If it returns :eof, it's all good, no
# timeout. Else, the first timeout was a proper timeout.
# This hack has to be done because io/wait#wait_readable doesn't provide a value for when
# the socket is closed by the server, and HTTP::Parser doesn't provide the limit for the chunks.
timeout = true unless @socket.to_io.wait_readable(remaining_time)
end
end
end

class Request
Expand Down
4 changes: 4 additions & 0 deletions app/serializers/rest/preview_card_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,8 @@ class REST::PreviewCardSerializer < ActiveModel::Serializer
def image
object.image? ? full_asset_url(object.image.url(:original)) : nil
end

def html
Sanitize.fragment(object.html, Sanitize::Config::MASTODON_OEMBED)
end
end
4 changes: 4 additions & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ class Application < Rails::Application
config.i18n.default_locale = :en
end

config.public_file_server.headers = {
'X-Content-Type-Options' => 'nosniff',
}

# config.paths.add File.join('app', 'api'), glob: File.join('**', '*.rb')
# config.autoload_paths += Dir[Rails.root.join('app', 'api', '*')]

Expand Down
27 changes: 27 additions & 0 deletions config/imagemagick/policy.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<policymap>
<!-- Set some basic system resource limits -->
<policy domain="resource" name="time" value="60" />

<policy domain="module" rights="none" pattern="URL" />

<policy domain="filter" rights="none" pattern="*" />

<!--
Ideally, we would restrict ImageMagick to only accessing its own
disk-backed pixel cache as well as Mastodon-created Tempfiles.

However, those paths depend on the operating system and environment
variables, so they can only be known at runtime.

Furthermore, those paths are not necessarily shared across Mastodon
processes, so even creating a policy.xml at runtime is impractical.

For the time being, only disable indirect reads.
-->
<policy domain="path" rights="none" pattern="@*" />

<!-- Disallow any coder by default, and only enable ones required by Mastodon -->
<policy domain="coder" rights="none" pattern="*" />
<policy domain="coder" rights="read | write" pattern="{PNG,JPEG,GIF,HEIC,WEBP}" />
<policy domain="coder" rights="write" pattern="{HISTOGRAM,RGB,INFO}" />
</policymap>
7 changes: 7 additions & 0 deletions config/initializers/paperclip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,10 @@ class NetworkingError < StandardError; end
end
end
end

# Set our ImageMagick security policy, but allow admins to override it
ENV['MAGICK_CONFIGURE_PATH'] = begin
imagemagick_config_paths = ENV.fetch('MAGICK_CONFIGURE_PATH', '').split(File::PATH_SEPARATOR)
imagemagick_config_paths << Rails.root.join('config', 'imagemagick').expand_path.to_s
imagemagick_config_paths.join(File::PATH_SEPARATOR)
end
3 changes: 3 additions & 0 deletions dist/nginx.conf
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,15 @@ server {
location ~ ^/(emoji|packs|system/accounts/avatars|system/media_attachments/files) {
add_header Cache-Control "public, max-age=31536000, immutable";
add_header Strict-Transport-Security "max-age=31536000" always;
add_header X-Content-Type-Options nosniff;
add_header Content-Security-Policy "default-src 'none'; form-action 'none'";
try_files $uri @proxy;
}

location /sw.js {
add_header Cache-Control "public, max-age=0";
add_header Strict-Transport-Security "max-age=31536000" always;
add_header X-Content-Type-Options nosniff;
try_files $uri @proxy;
}

Expand Down
5 changes: 1 addition & 4 deletions lib/paperclip/transcoder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ def initialize(file, options = {}, attachment = nil)
def make
metadata = VideoMetadataExtractor.new(@file.path)

unless metadata.valid?
Paperclip.log("Unsupported file #{@file.path}")
return File.open(@file.path)
end
raise Paperclip::Error, "Error while transcoding #{@file.path}: unsupported file" unless metadata.valid?

update_attachment_type(metadata)
update_options_from_metadata(metadata)
Expand Down
22 changes: 11 additions & 11 deletions lib/sanitize_ext/sanitize_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,26 +132,26 @@ module Config
]
)

MASTODON_OEMBED ||= freeze_config merge(
RELAXED,
elements: RELAXED[:elements] + %w(audio embed iframe source video),
MASTODON_OEMBED ||= freeze_config(
elements: %w(audio embed iframe source video),

attributes: merge(
RELAXED[:attributes],
attributes: {
'audio' => %w(controls),
'embed' => %w(height src type width),
'iframe' => %w(allowfullscreen frameborder height scrolling src width),
'source' => %w(src type),
'video' => %w(controls height loop width),
'div' => [:data]
),
},

protocols: merge(
RELAXED[:protocols],
protocols: {
'embed' => { 'src' => HTTP_PROTOCOLS },
'iframe' => { 'src' => HTTP_PROTOCOLS },
'source' => { 'src' => HTTP_PROTOCOLS }
)
'source' => { 'src' => HTTP_PROTOCOLS },
},

add_attributes: {
'iframe' => { 'sandbox' => 'allow-scripts allow-same-origin allow-popups allow-popups-to-escape-sandbox allow-forms' },
}
)
end
end