Security issue in FlyingSocks `Socket.write`

[stackotter]

Overview

If the data passed to Socket.write is a slice with a non-zero startIndex, memory after the end of the data buffer will be leaked to the recipient.

Cause

The issue is on line 229 of Socket.swift:

let sent = try write(buffer.baseAddress! + index, length: data.endIndex - index)

The code assumes that buffer.baseAddress! + index correctly gets the byte at index in the data, however baseAddress points to the byte at startIndex not at index 0. For example, consider the following code:

let data = "abcd".data(using: .utf8)!
let slice = data[2...] // contains "cd"

let index = slice.startIndex
try slice.withUnsafeBytes { buffer in               
    _ = try write(buffer.baseAddress! + index, length: data.endIndex - index)
    //                   (1)            (2)            (3)
    //
    // 1. baseAddress points to "c"
    // 2. after adding startIndex (which is 2), the pointer points to the byte after the end of the buffer
    // 3. length is 2 (4 - 2)
    //
    // In this example scenario, the server accidentally sends two bytes of
    // the memory after the end of the data buffer to the client, which could
    // lead to sensitive data being leaked in certain setups. It could also potentially
    // be combined with certain other types of vulnerabilities to execute arbitrary code.
}

Proof of concept

First, run the following command to start a tcp listener that simply prints out any data it receives.

nc -l 8080

Next, run this code snippet with swift run for the highest chance of reproducing, because that's how I ran it:

@main
struct FlyingFoxDataTest {
    static func main() async throws {
        // Generate some dummy data and make a slice
        let data = String(repeating: "abcd", count: 32).data(using: .utf8)!
        let slice = data[64...]
        
        // This length of string seems to work consistently on my laptop
        let secretPassword = "thisismyverylongandsecuresecretpasswordthisismyverylongandsecuresecretpasswordthisismyverylongandsecuresecretpassworditissogood!".data(using: .utf8)!
        
        // Attempt to send the slice through the socket
        let socket = try await AsyncSocket.connected(to: .inet(ip4: "127.0.0.1", port: 8080), pool: .polling)
        try await socket.write(slice)
    }
}

When I run that snippet, I get the following output:

$ nc -l 8080
thisismyverylongandsecuresecretpasswordthisismyverylongandsecure

As you can see, it sent the first half of secretPassword instead of the contents of the data slice. This bug could have pretty bad side-effects if it appeared in any unfortunate situations.

Mitigation

Make the following change:

- let sent = try write(buffer.baseAddress! + index, length: data.endIndex - index)
+ let sent = try write(buffer.baseAddress! + index - data.startIndex, length: data.endIndex - index)

I'll make a PR to fix this soon.

And I know this full on bug report might be a bit overkill, but I had fun getting that proof of concept to work, so I did it anyway.

[swhitty]

Thank you for the great explanation and fix. I love learning about this stuff.

[stackotter]

You're welcome! I also enjoyed writing the explanation and creating the proof of concept, so it's a win win :)