v9.4.0+ creates regression on Media Urls loading in backoffice via GetBigThumbnail

[brentheimer]

Which exact Umbraco version are you using? For example: 9.0.1 - don't just write v9

9.4.0

Bug summary

PR https://github.com/umbraco/Umbraco-CMS/pull/11606 introduced code to only allow relative urls to prevent open redirects. This prevents using absolute urls, causing any image previews in backoffice to not render the image. In my scenario, I'm using a custom IFileSystem connecting to Cloudinary service and storing urls as absolute.

(Previously showed thumbnail previews)

A few lines later this exception comment appears

catch (Exception)
{
    // if we get an exception here it's probably because the image path being requested is an image that doesn't exist
    // in the local media file system. This can happen if someone is storing an absolute path to an image online, which
    // is perfectly legal but in that case the media file system isn't going to resolve it.
    // so ignore and we won't set a last modified date.
}

this will never happen, since before this code absolute urls are rejected.

Since this method doesn't interact with the registered IFileSystem I can't fix without creating a PR. However this still represents a regression for other possible overrides of default IFileSystem.

If preventing open redirects on this path is critical, possible considered solutions are:

1. creating a safelist of domains, updating code to check that list (possibly as property on registered IMediaUrlGenerator, appsettings, etc.)
2. Updating IMediaUrlProvider to allow for handling thumbnail generation. shifting current implementation to Umbraco.Cms.Core.Routing.DefaultMediaUrlProvider.

Specifics

Occurs on all browsers.

Steps to reproduce

Implement a IFileSystem that allows storing absolute urls. Add an image to an existing property that implements DataType of MediaPicker Look under Content dashboard on page node for that property. You will see just the name. Network tools will show a 401 status code returned.

Expected result / actual result

You will see just the name. Network tools will show a 401 status code returned. It should return the full image thumbnail as in previous versions.

[lordy1981]

I am having this same issue, so we can create a custom file system, but the back office doesn't support that 100% ?

[lordy1981]

@brentheimer did you find a way around this ?

[brentheimer]

@lordy1981 Currently I'm using Middleware to intercept http calls that match the thumbnail request. Not ideal, but it works well. As a bonus svg thumbnails work now too.

[lordy1981]

@brentheimer do you have an example you can share?

[brentheimer]

@lordy1981 sure. Note that this is specific to my IFileSystem provider, you'd have to update for your own, and be sure yours is registered for DI. It's also not fully optimized, as it was a stop-gap until this bug was fixed.

internal class CloudinaryMiddleware : IMiddleware
    {
        private readonly ILogger<CloudinaryMiddleware> _logger;
        private readonly ICloudinaryFileSystem _cloudinaryFileSystem;

        /// <summary>
        /// Provides middleware interception for Cloudinary services
        /// </summary>
        /// <remarks>
        /// sample path: //https://localhost:44358/umbraco/backoffice/umbracoapi/images/GetBigThumbnail?originalImagePath=/i-am-error.png&rnd=0.2646908805700101
        /// </remarks>
        private const string PATH_TO_INTERCEPT = "/umbraco/backoffice/umbracoapi/images/GetBigThumbnail";

        public CloudinaryMiddleware(ILogger<CloudinaryMiddleware> logger, ICloudinaryFileSystem cloudinaryFileSystem)
        {
            _logger = logger;
            _cloudinaryFileSystem = cloudinaryFileSystem;
        }

        public async Task InvokeAsync(HttpContext context, RequestDelegate next)
        {
            // check if media
            if (!context.Request.Path.StartsWithSegments(PATH_TO_INTERCEPT, StringComparison.InvariantCultureIgnoreCase))
            {
                await next(context).ConfigureAwait(false);
                return;
            }

            // alter originalImagePath, return payload
            string? mediaPath = context.Request.Query["originalImagePath"];
            if(string.IsNullOrEmpty(mediaPath))
            {
                _logger.LogWarning("Bad requestU for url {url}", context.Request.GetEncodedUrl());
            }

            // use filesystem to generate thumbnail -- non-standard method added for this purpose.
            string thumbnailPath = _cloudinaryFileSystem.GetThumbnailImageUrl(mediaPath, 320);

            // respond
            // you need to have middleware early otherwise you should have checks here to see if response has started yet.
            context.Response.Clear();
            context.Response.StatusCode = StatusCodes.Status302Found;
            // set location to let browser load it from absolute path provided
            context.Response.Headers["location"] = thumbnailPath;
        }
    }

[kjac]

Hi @brentheimer,

Thank you for reaching out, and sorry for the late response. Also thanks a lot for providing a workaround 💪

I agree this is something that should be fixed.

For the sake of backoffice security don't want to reintroduce open redirects, so I think your proposed solution 1 sounds really great:

1. creating a safelist of domains, updating code to check that list (possibly as property on registered IMediaUrlGenerator, appsettings, etc.)

We would love some help with this, so I'm marking this as up for grabs for the community 👍

[github-actions[bot]]

Hi @brentheimer,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post.

Thanks muchly, from your friendly Umbraco GitHub bot :-)

[nikolajlauridsen]

This has been resolved in #13900 and #13962. Thanks for reporting 🐛 👍