arbitrary code execution when GOMAXPROCS>1

[kortschak]

With the current setup it is possible to execute arbitrary code if GOMAXPROCS>1 or can be set to a value greater than 1 (i.e. import of runtime is allowed). By default no package import is allowed, but there should be a warning to the effect that allowing runtime is unsafe when presented with untrusted code.

Running the code below shows that when GOMAXPROCS>1 a []byte can be converted to a func() without any import. No arbitrary code is included, but a malicious payload would be trivial to include.

package main

func sliceToFunc(b []byte) (f func()) {
    var i, j, k interface{}
    i = f
    j = &b[0]

    done := false
    go func() {
        for !done {
            k = i
            k = j
        }
    }()
    for {
        if p, ok := k.(func()); ok && p != nil {
            f = p
            done = true
            break
        }
    }

    return
}

func main() {
    arbCode := []byte{0} // Arbitrary code goes here.
    f := sliceToFunc(arbCode)
    f()
}

[zond]

Urgh, I didn't even know about this way to break the type safety :O

Give me a merge request that warns in a way you find appropriate and I will merge it.

Sorry about not seeing this sooner, I think my github notifications are wonky.

[kortschak]

I'm not exactly sure how you'd like to deal with it. A perfectly adequate approach would be to put a prominent warning on the README.md and in the package preamble that this is possible - link to the article by Russ since it's informative anyway and learning is always good.

A more complex solution would be to allow runtime imports but go over the AST of the presented code to find calls to runtime.GOMAXPROCS and remove those nodes (or set the parameter to 1 - this is probably better since it may be used an expression in client code). This might be a worthwhile longer term solution, but the warning can sit in place until that happens.

[zond]

Both solutions are interesting.

Since I am no longer using gosafe, and spending all my time on other projects, would you like to send a pull request with a solution that you are happy with providing and using? :)

[kortschak]

I think that it's probably up to you to provide the warning documentation; I don't use gosafe and I don't really have enough time to work on a proper fix, although there is a prospect of both of those in the future.

If/When I do put it into use I will implement the second approach if that has not already been done. Until that protection is in place (even with documentation) this issue should probably remain open so that other approaches can be raised: for example the one used in the playground which involves revisions to the runtime (this is less attractive in this situation, but should probably be considered).

[zond]

Right. I pushed c291460 that documents this in the README, and makes importing "runtime" require using "AllowRuntime" instead of the regular "Allow". This will have to be enough for now.