Compare commits

...

10 Commits

Author SHA1 Message Date
Claire d13351afe8
Merge pull request from GHSA-55j9-c3mp-6fcq
ci/woodpecker/push/woodpecker Pipeline was successful Details
2023-07-07 13:42:05 +10:00
Claire 5f1d9ad0c6
Merge pull request from GHSA-9pxv-6qvf-pjwc
* Fix timeout handling of outbound HTTP requests

* Use CLOCK_MONOTONIC instead of Time.now
2023-07-07 13:42:05 +10:00
Claire 912582b198
Merge pull request from GHSA-9928-3cp5-93fm
* Fix attachments getting processed despite failing content-type validation

* Add a restrictive ImageMagick security policy tailored for Mastodon

* Fix misdetection of MP3 files with large cover art

* Reject unprocessable audio/video files instead of keeping them unchanged
2023-07-07 13:42:05 +10:00
Claire 56d7c5badc
Merge pull request from GHSA-ccm4-vgcc-73hp
* Tighten allowed HTML in oEmbed-based preview cards

* Sanitize preview cards at render time

* Add `sandbox` attribute to preview card iframes
2023-07-07 13:42:05 +10:00
Claire ff0a267d03
Add hardened headers to user-uploaded files (#25756) 2023-07-07 13:42:05 +10:00
Eugen Rochko b21fe63102
Add canonical link tags in web UI (#25715) 2023-07-07 13:42:05 +10:00
Eugen Rochko e749599432
Add button to see results for polls in web UI (#25726) 2023-07-07 13:42:04 +10:00
Claire 648aeb7c97
Fix OAuth apps page crashing when listing apps with certain admin API scopes (#25713) 2023-07-07 13:42:04 +10:00
Claire 994c2c9402
Fix re-activated accounts being deleted by AccountDeletionWorker (#25711) 2023-07-07 13:42:04 +10:00
Trevor Wolf a95a453711
fix read more button overlapping thread line bug (#25706) 2023-07-07 13:42:04 +10:00
22 changed files with 190 additions and 40 deletions

View File

@ -64,6 +64,10 @@ module FormattingHelper
end end
def account_field_value_format(field, with_rel_me: true) def account_field_value_format(field, with_rel_me: true)
html_aware_format(field.value, field.account.local?, with_rel_me: with_rel_me, with_domains: true, multiline: false) if field.verified? && !field.account.local?
TextFormatter.shortened_link(field.value_for_verification)
else
html_aware_format(field.value, field.account.local?, with_rel_me: with_rel_me, with_domains: true, multiline: false)
end
end end
end end

View File

@ -130,6 +130,10 @@ class Poll extends ImmutablePureComponent {
this.props.refresh(); this.props.refresh();
}; };
handleReveal = () => {
this.setState({ revealed: true });
}
renderOption (option, optionIndex, showResults) { renderOption (option, optionIndex, showResults) {
const { poll, lang, disabled, intl } = this.props; const { poll, lang, disabled, intl } = this.props;
const pollVotesCount = poll.get('voters_count') || poll.get('votes_count'); const pollVotesCount = poll.get('voters_count') || poll.get('votes_count');
@ -205,14 +209,14 @@ class Poll extends ImmutablePureComponent {
render () { render () {
const { poll, intl } = this.props; const { poll, intl } = this.props;
const { expired } = this.state; const { revealed, expired } = this.state;
if (!poll) { if (!poll) {
return null; return null;
} }
const timeRemaining = expired ? intl.formatMessage(messages.closed) : <RelativeTimestamp timestamp={poll.get('expires_at')} futureDate />; const timeRemaining = expired ? intl.formatMessage(messages.closed) : <RelativeTimestamp timestamp={poll.get('expires_at')} futureDate />;
const showResults = poll.get('voted') || expired; const showResults = poll.get('voted') || revealed || expired;
const disabled = this.props.disabled || Object.entries(this.state.selected).every(item => !item); const disabled = this.props.disabled || Object.entries(this.state.selected).every(item => !item);
let votesCount = null; let votesCount = null;
@ -231,9 +235,10 @@ class Poll extends ImmutablePureComponent {
<div className='poll__footer'> <div className='poll__footer'>
{!showResults && <button className='button button-secondary' disabled={disabled || !this.context.identity.signedIn} onClick={this.handleVote}><FormattedMessage id='poll.vote' defaultMessage='Vote' /></button>} {!showResults && <button className='button button-secondary' disabled={disabled || !this.context.identity.signedIn} onClick={this.handleVote}><FormattedMessage id='poll.vote' defaultMessage='Vote' /></button>}
{showResults && !this.props.disabled && <span><button className='poll__link' onClick={this.handleRefresh}><FormattedMessage id='poll.refresh' defaultMessage='Refresh' /></button> · </span>} {!showResults && <><button className='poll__link' onClick={this.handleReveal}><FormattedMessage id='poll.reveal' defaultMessage='See results' /></button> · </>}
{showResults && !this.props.disabled && <><button className='poll__link' onClick={this.handleRefresh}><FormattedMessage id='poll.refresh' defaultMessage='Refresh' /></button> · </>}
{votesCount} {votesCount}
{poll.get('expires_at') && <span> · {timeRemaining}</span>} {poll.get('expires_at') && <> · {timeRemaining}</>}
</div> </div>
</div> </div>
); );

View File

@ -476,6 +476,7 @@ class Header extends ImmutablePureComponent {
<Helmet> <Helmet>
<title>{titleFromAccount(account)}</title> <title>{titleFromAccount(account)}</title>
<meta name='robots' content={(isLocal && isIndexable) ? 'all' : 'noindex'} /> <meta name='robots' content={(isLocal && isIndexable) ? 'all' : 'noindex'} />
<link rel='canonical' href={account.get('url')} />
</Helmet> </Helmet>
</div> </div>
); );

View File

@ -716,6 +716,7 @@ class Status extends ImmutablePureComponent {
<Helmet> <Helmet>
<title>{titleFromStatus(intl, status)}</title> <title>{titleFromStatus(intl, status)}</title>
<meta name='robots' content={(isLocal && isIndexable) ? 'all' : 'noindex'} /> <meta name='robots' content={(isLocal && isIndexable) ? 'all' : 'noindex'} />
<link rel='canonical' href={status.get('url')} />
</Helmet> </Helmet>
</Column> </Column>
); );

View File

@ -487,6 +487,7 @@
"picture_in_picture.restore": "Put it back", "picture_in_picture.restore": "Put it back",
"poll.closed": "Closed", "poll.closed": "Closed",
"poll.refresh": "Refresh", "poll.refresh": "Refresh",
"poll.reveal": "See results",
"poll.total_people": "{count, plural, one {# person} other {# people}}", "poll.total_people": "{count, plural, one {# person} other {# people}}",
"poll.total_votes": "{count, plural, one {# vote} other {# votes}}", "poll.total_votes": "{count, plural, one {# vote} other {# votes}}",
"poll.vote": "Vote", "poll.vote": "Vote",

View File

@ -1127,6 +1127,8 @@ body > [data-popper-placement] {
} }
&--in-thread { &--in-thread {
$thread-margin: 46px + 10px;
border-bottom: 0; border-bottom: 0;
.status__content, .status__content,
@ -1137,8 +1139,12 @@ body > [data-popper-placement] {
.attachment-list, .attachment-list,
.picture-in-picture-placeholder, .picture-in-picture-placeholder,
.status-card { .status-card {
margin-inline-start: 46px + 10px; margin-inline-start: $thread-margin;
width: calc(100% - (46px + 10px)); width: calc(100% - ($thread-margin));
}
.status__content__read-more-button {
margin-inline-start: $thread-margin;
} }
} }

View File

@ -7,11 +7,48 @@ require 'resolv'
# Monkey-patch the HTTP.rb timeout class to avoid using a timeout block # 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 # around the Socket#open method, since we use our own timeout blocks inside
# that method # 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 class HTTP::Timeout::PerOperation
def connect(socket_class, host, port, nodelay = false) def connect(socket_class, host, port, nodelay = false)
@socket = socket_class.open(host, port) @socket = socket_class.open(host, port)
@socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay @socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay
end 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 end
class Request class Request

View File

@ -1,7 +1,7 @@
# frozen_string_literal: true # frozen_string_literal: true
class ScopeParser < Parslet::Parser class ScopeParser < Parslet::Parser
rule(:term) { match('[a-z]').repeat(1).as(:term) } rule(:term) { match('[a-z_]').repeat(1).as(:term) }
rule(:colon) { str(':') } rule(:colon) { str(':') }
rule(:access) { (str('write') | str('read')).as(:access) } rule(:access) { (str('write') | str('read')).as(:access) }
rule(:namespace) { str('admin').as(:namespace) } rule(:namespace) { str('admin').as(:namespace) }

View File

@ -48,6 +48,26 @@ class TextFormatter
html.html_safe # rubocop:disable Rails/OutputSafety html.html_safe # rubocop:disable Rails/OutputSafety
end end
class << self
include ERB::Util
def shortened_link(url, rel_me: false)
url = Addressable::URI.parse(url).to_s
rel = rel_me ? (DEFAULT_REL + %w(me)) : DEFAULT_REL
prefix = url.match(URL_PREFIX_REGEX).to_s
display_url = url[prefix.length, 30]
suffix = url[prefix.length + 30..-1]
cutoff = url[prefix.length..-1].length > 30
<<~HTML.squish
<a href="#{h(url)}" target="_blank" rel="#{rel.join(' ')}" translate="no"><span class="invisible">#{h(prefix)}</span><span class="#{cutoff ? 'ellipsis' : ''}">#{h(display_url)}</span><span class="invisible">#{h(suffix)}</span></a>
HTML
rescue Addressable::URI::InvalidURIError, IDN::Idna::IdnaError
h(url)
end
end
private private
def rewrite def rewrite
@ -70,19 +90,7 @@ class TextFormatter
end end
def link_to_url(entity) def link_to_url(entity)
url = Addressable::URI.parse(entity[:url]).to_s TextFormatter.shortened_link(entity[:url], rel_me: with_rel_me?)
rel = with_rel_me? ? (DEFAULT_REL + %w(me)) : DEFAULT_REL
prefix = url.match(URL_PREFIX_REGEX).to_s
display_url = url[prefix.length, 30]
suffix = url[prefix.length + 30..-1]
cutoff = url[prefix.length..-1].length > 30
<<~HTML.squish
<a href="#{h(url)}" target="_blank" rel="#{rel.join(' ')}" translate="no"><span class="invisible">#{h(prefix)}</span><span class="#{cutoff ? 'ellipsis' : ''}">#{h(display_url)}</span><span class="invisible">#{h(suffix)}</span></a>
HTML
rescue Addressable::URI::InvalidURIError, IDN::Idna::IdnaError
h(entity[:url])
end end
def link_to_hashtag(entity) def link_to_hashtag(entity)

View File

@ -22,15 +22,14 @@ module Attachmentable
included do included do
def self.has_attached_file(name, options = {}) # rubocop:disable Naming/PredicateName def self.has_attached_file(name, options = {}) # rubocop:disable Naming/PredicateName
options = { validate_media_type: false }.merge(options)
super(name, options) super(name, options)
send(:"before_#{name}_post_process") do
send(:"before_#{name}_validate") do
attachment = send(name) attachment = send(name)
check_image_dimension(attachment) check_image_dimension(attachment)
set_file_content_type(attachment) set_file_content_type(attachment)
obfuscate_file_name(attachment) obfuscate_file_name(attachment)
set_file_extension(attachment) set_file_extension(attachment)
Paperclip::Validators::MediaTypeSpoofDetectionValidator.new(attributes: [name]).validate(self)
end end
end end
end end

View File

@ -11,4 +11,8 @@ class REST::PreviewCardSerializer < ActiveModel::Serializer
def image def image
object.image? ? full_asset_url(object.image.url(:original)) : nil object.image? ? full_asset_url(object.image.url(:original)) : nil
end end
def html
Sanitize.fragment(object.html, Sanitize::Config::MASTODON_OEMBED)
end
end end

View File

@ -6,9 +6,12 @@ class AccountDeletionWorker
sidekiq_options queue: 'pull', lock: :until_executed sidekiq_options queue: 'pull', lock: :until_executed
def perform(account_id, options = {}) def perform(account_id, options = {})
account = Account.find(account_id)
return unless account.suspended?
reserve_username = options.with_indifferent_access.fetch(:reserve_username, true) reserve_username = options.with_indifferent_access.fetch(:reserve_username, true)
skip_activitypub = options.with_indifferent_access.fetch(:skip_activitypub, false) skip_activitypub = options.with_indifferent_access.fetch(:skip_activitypub, false)
DeleteAccountService.new.call(Account.find(account_id), reserve_username: reserve_username, skip_activitypub: skip_activitypub, reserve_email: false) DeleteAccountService.new.call(account, reserve_username: reserve_username, skip_activitypub: skip_activitypub, reserve_email: false)
rescue ActiveRecord::RecordNotFound rescue ActiveRecord::RecordNotFound
true true
end end

View File

@ -28,6 +28,7 @@ require_relative '../lib/paperclip/url_generator_extensions'
require_relative '../lib/paperclip/attachment_extensions' require_relative '../lib/paperclip/attachment_extensions'
require_relative '../lib/paperclip/lazy_thumbnail' require_relative '../lib/paperclip/lazy_thumbnail'
require_relative '../lib/paperclip/gif_transcoder' require_relative '../lib/paperclip/gif_transcoder'
require_relative '../lib/paperclip/media_type_spoof_detector_extensions'
require_relative '../lib/paperclip/transcoder' require_relative '../lib/paperclip/transcoder'
require_relative '../lib/paperclip/type_corrector' require_relative '../lib/paperclip/type_corrector'
require_relative '../lib/paperclip/response_with_limit_adapter' require_relative '../lib/paperclip/response_with_limit_adapter'

View File

@ -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>

View File

@ -153,3 +153,10 @@ unless defined?(Seahorse)
end end
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

2
dist/nginx.conf vendored
View File

@ -109,6 +109,8 @@ server {
location ~ ^/system/ { location ~ ^/system/ {
add_header Cache-Control "public, max-age=2419200, immutable"; add_header Cache-Control "public, max-age=2419200, immutable";
add_header Strict-Transport-Security "max-age=63072000; includeSubDomains"; add_header Strict-Transport-Security "max-age=63072000; includeSubDomains";
add_header X-Content-Type-Options nosniff;
add_header Content-Security-Policy "default-src 'none'; form-action 'none'";
try_files $uri =404; try_files $uri =404;
} }

View File

@ -0,0 +1,22 @@
# frozen_string_literal: true
module Paperclip
module MediaTypeSpoofDetectorExtensions
def calculated_content_type
return @calculated_content_type if defined?(@calculated_content_type)
@calculated_content_type = type_from_file_command.chomp
# The `file` command fails to recognize some MP3 files as such
@calculated_content_type = type_from_marcel if @calculated_content_type == 'application/octet-stream' && type_from_marcel == 'audio/mpeg'
@calculated_content_type
end
def type_from_marcel
@type_from_marcel ||= Marcel::MimeType.for Pathname.new(@file.path),
name: @file.path
end
end
end
Paperclip::MediaTypeSpoofDetector.prepend(Paperclip::MediaTypeSpoofDetectorExtensions)

View File

@ -19,10 +19,7 @@ module Paperclip
def make def make
metadata = VideoMetadataExtractor.new(@file.path) metadata = VideoMetadataExtractor.new(@file.path)
unless metadata.valid? raise Paperclip::Error, "Error while transcoding #{@file.path}: unsupported file" unless metadata.valid?
Paperclip.log("Unsupported file #{@file.path}")
return File.open(@file.path)
end
update_attachment_type(metadata) update_attachment_type(metadata)
update_options_from_metadata(metadata) update_options_from_metadata(metadata)

View File

@ -32,6 +32,11 @@ class PublicFileServerMiddleware
end end
end end
# Override the default CSP header set by the CSP middleware
headers['Content-Security-Policy'] = "default-src 'none'; form-action 'none'" if request_path.start_with?(paperclip_root_url)
headers['X-Content-Type-Options'] = 'nosniff'
[status, headers, response] [status, headers, response]
end end

View File

@ -107,26 +107,26 @@ class Sanitize
] ]
) )
MASTODON_OEMBED ||= freeze_config merge( MASTODON_OEMBED ||= freeze_config(
RELAXED, elements: %w(audio embed iframe source video),
elements: RELAXED[:elements] + %w(audio embed iframe source video),
attributes: merge( attributes: {
RELAXED[:attributes],
'audio' => %w(controls), 'audio' => %w(controls),
'embed' => %w(height src type width), 'embed' => %w(height src type width),
'iframe' => %w(allowfullscreen frameborder height scrolling src width), 'iframe' => %w(allowfullscreen frameborder height scrolling src width),
'source' => %w(src type), 'source' => %w(src type),
'video' => %w(controls height loop width), 'video' => %w(controls height loop width),
'div' => [:data] },
),
protocols: merge( protocols: {
RELAXED[:protocols],
'embed' => { 'src' => HTTP_PROTOCOLS }, 'embed' => { 'src' => HTTP_PROTOCOLS },
'iframe' => { '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' },
}
) )
LINK_REL_TRANSFORMER = lambda do |env| LINK_REL_TRANSFORMER = lambda do |env|

BIN
spec/fixtures/files/boop.mp3 vendored Normal file

Binary file not shown.

View File

@ -152,6 +152,26 @@ RSpec.describe MediaAttachment, paperclip_processing: true do
end end
end end
describe 'mp3 with large cover art' do
let(:media) { described_class.create(account: Fabricate(:account), file: attachment_fixture('boop.mp3')) }
it 'detects it as an audio file' do
expect(media.type).to eq 'audio'
end
it 'sets meta for the duration' do
expect(media.file.meta['original']['duration']).to be_within(0.05).of(0.235102)
end
it 'extracts thumbnail' do
expect(media.thumbnail.present?).to be true
end
it 'gives the file a random name' do
expect(media.file_file_name).to_not eq 'boop.mp3'
end
end
describe 'jpeg' do describe 'jpeg' do
let(:media) { described_class.create(account: Fabricate(:account), file: attachment_fixture('attachment.jpg')) } let(:media) { described_class.create(account: Fabricate(:account), file: attachment_fixture('attachment.jpg')) }