Crash in clipboardFiles.node when copying from certain other apps

[MichaelBuckley]

Preflight Checklist

• [x] I have installed the latest version of Storage Explorer.
• [x] I have checked existing resources, including the troubleshooting guide and the release notes.
• [x] I have searched for similar issues.

Storage Explorer Version

1.36.2

Regression From

No response

Architecture

arm64

Storage Explorer Build Number

20241029.2

Platform

macOS

OS Version

macOS 14 and 15, possibly earlier versions too

Bug Description

Storage Explorer will crash as soon as data another app with a specific structure is placed in the system-wide pasteboard.

Steps to Reproduce

1. Launch Storage Explorer
2. Launch Transmit 5.10.5 ( https://panic.com/transmit/ ) (There is a free trial period which can be use to test this bug.)
3. Connect to a server in Transmit.
4. Select a file on the server in Transmit
5. Edit > Copy or command-c

Actual Experience

Storage Explorer crashes. Here is a sample crash log. Microsoft Azure Storage Explorer-2024-11-05-125837.txt

Expected Experience

Storage Explorer does not crash.

Additional Context

The crash appears to occur when Storage Explorer attempts to parse the data placed on the macOS pasteboard. Some other apps can't process the data, but Storage Explorer is the only app we found that crashes. We here at Panic have found a solution and will release an update to Transmit that won't trigger the crash, other apps could cause Storage Explorer to crash if they use the pasteboard the same way.

We suspect there may be a bug in macOS causing a problem with the data to be written to the pasteboard. We're still gathering information and will follow up with Apple if so.

The issue itself is very strange. Transmit creates an NSFilePromiseProvider for the file and places it on the pasteboard. Transmit also also attaches a userInfo dictionary to this NSFilePromiseProvider. As long as the userInfo dictionary remains flat, Storage Explorer will not crash. However, if another dictionary is added as a value to this userInfo dictionary, Storage Explorer will crash.

The userInfo dictionary ought to only be available to Transmit. The fact that this affects the pasteboard for other apps is surprising to us.

[MichaelBuckley]

Here's a more readable crash report. Sorry about the last one. Microsoft.Azure.Storage.Explorer-2024-11-05-125837-translated.txt

[craxal]

@JasonYeMSFT Can you look into this? If you don't have time feel free to reassign.

[JasonYeMSFT]

@MichaelBuckley Could you share a minimum sample data that can be used to reproduce the bug? If it's not plain text, you can share a piece of code that can be used to put that piece of data into the clipboard.

Our code attempts to get the files copied to the clipboard by getting their full paths. The core logic is done by the following function

std::vector<std::string> getData() {
        // Get reference to the general system pasteboard
        NSPasteboard *pb = [NSPasteboard generalPasteboard];
        // Specify to only show NSURL type objects on the pasteboard
        NSArray *classes = [[[NSArray alloc] initWithObjects:[NSURL class], nil] autorelease];
        // default option
        NSDictionary *options = [NSDictionary dictionary];
        NSArray *copiedItems = [pb readObjectsForClasses:classes options:options];

        std::vector<std::string> pathList;
        if (copiedItems != nil) {
            for (unsigned int i = 0; i < copiedItems.count; i++) {
                NSObject *s = copiedItems[i];
                std::string filePathUrl(s.description.UTF8String);
                // Skip the "file://" prefix
                std::string filePath = filePathUrl.substr(7);
                // convert the encoded URI to raw byte sequence so that v8 can properly decode it
                std::string decodedFilePath = decodeURIComponent(filePath);
                pathList.push_back(decodedFilePath);
            }
        }

        return pathList;
}

If you can spot an opportunity for us to protect our code from crashing, could you please give us some advice on how to achieve it?

[MichaelBuckley]

@JasonYeMSFT I will try to put together a minimally-reproducible sample today, but just from looking at that code and the stack trace in the crash, I'm almost entirely certain that the problem is calling std::string::substr(7) without checking that the string is at least 7 characters long. When the position parameter for substr is greater than the string length, it throws std::out_of_bounds, which is what we see in the stack trace.

Exactly what < 7 length URL you're getting there when we put an NSDictionary in an NSDictionary, I'm not sure, but I'll find out shortly.

[MichaelBuckley]

That is indeed the problem. The URL is an empty string in this case, and the crash occurs in the std::string::substr call.

This appears to be a bug in macOS itself. Calling [pb types] in the code above returns slightly different types. When copying from the version of Transmit that causes the crash, the types list includes public.file-url, but the actual URL is an https:// URL. We're not copying a local file, so it would make sense that's an empty URL. However, when copying from the fixed version of Transmit, the types list instead contains public.url.

I'll still see if I can get a minimally-reproducible case—the code in Transmit is not trivial—and I'll follow up with Apple, but you might want to add a check before you call substr.

[MichaelBuckley]

Update for anyone who finds this in a Google search years down the road:

There was no bug in macOS. Transmit had a piece of code that was looking for data in the outer userInfo dictionary when it should have been looking in the inner userInfo dictionary. There's noting inherently wrong with nesting dictionaries. But as a result of the bug in Transmit, it was placing an empty string as a URL on the pasteboard. I'm attaching a sample project which you can build in Xcode and click a button to recreate how Transmit was putting the empty URL on the pasteboard.

PasteTest.zip

[JasonYeMSFT]

I merged a change to check for the url string to prevent the out of range error.