Swift: Query for escaping parameters of unsafe closures

[rdmarsh2]

No description provided.

[geoffw0]

Good early results, this looks promising.

[geoffw0]

Another variant here would be calling a member function that stores the pointer argument in the object it's called on for later use. The effect is similar to the v.field = p case.

[geoffw0]

This looks like a cartesian product - might be a performance issue.

[rdmarsh2]

Yes, it is (and I hit the performance issue on a real database). I've shrunk it by narrowing the sinks.

[github-actions]

QHelp previews:

swift/ql/src/queries/Security/CWE-825/UnsafePointerEscapesClosure.qhelp

Unsafe pointer escapes closure

Certain functions take a closure and pass a temporary pointer into it. If this pointer escapes from the closure and is used outside it, memory corruption may occur.

Recommendation

Do not use temporary pointers outside the closure they are passed to.

Example

In the first example below, the pointer is returned from the closure, potentially leading to memory corruption. In the second example, all work with the pointer is done inside the closure, and it is not returned.

func bad() {
    let ints = [1,2,3]
    let bytes = ints.withUnsafeBytes{
        return $0
    }
    print(bytes)
}

func good() {
    let ints = [1,2,3]
    let bytes = ints.withUnsafeBytes{
        print($0)
    }
}

References

• withUnsafeBytes
• Common Weakness Enumeration: CWE-825.

[geoffw0]

Is it safe that this can match arguments other than the intended closure? If it's possible to pass a closure or function to another argument that is.

[rdmarsh2]

We might need to revisit this if there's a case where the function takes multiple closures, but everything I've modeled only takes one closure argument.

[geoffw0]

What's the issue with implicit returns? Surely we will see them in real world code.

[rdmarsh2]

The issue is that we were picking up the implicit return of the assignment, but not the field being assigned. This is currently not detected... I think there's some missing flow for closure captures.

[geoffw0]

Needs precision, severity and security-severity tags. I guess we want to test with a near-final version before choosing a precision.

[geoffw0]

What do you think about linking to https://www.kodeco.com/7181017-unsafe-swift-using-pointers-and-interacting-with-c as well?