After writing Go for years, many of us have learned the error-checking pattern down to our bones: “Does this function return an error? Ope, better make sure it’s nil before moving on.”
And that’s great! This should be our default behavior when writing Go.
However, rote error checking can sometimes prevent critical thinking about what that error actually means to the code: When does that function return an error? And does it encompass everything you think it does?
For instance, in os.Create, the nil error value can trick you into thinking you’re safe with file creation. Reading the linked documentation, os.Create actually truncates the file when it already exists instead of throwing any indication that it’s not a new file.
This leaves us vulnerable to a symlink attack.
Does it exist?
Say my program needs to create and use a file. Almost every example of idiomatic Go code guides us through an error check, but no validation of whether the file existed before Create was called. If a symbolic link had already been set up for that file, no error would occur, but the file and its contents would not behave as intended due to the truncation behavior. The risk is that we can remove information using the program to overwrite it for us.
At Trail of Bits, this issue comes up frequently in audits. Thankfully, the fix for it is incredibly easy. We just need to check if a file exists before attempting to create it. A slight tweak in our approach to idiomatic Go can make file creation safer long term and take us one step closer to prioritizing security in Go programs.
The situation
Let’s say there’s a file, my_logs, that I need to create and write to. However, in another part of the codebase, someone previously set up a symlink with ln -s other/logs my_logs.
- logs - notes - things I care about - very important information we can't lose
Contents of other/logs.
package mainimport (
“fmt”
“os”
)func main() {
file, err := os.Create(“my_logs”)
if err != nil {
fmt.Printf(“Error creating file: %s”, err)
}_, err = file.Write([]byte("My logs for this process")) if err != nil { fmt.Println(err) }}
symlink_attack.go.
$ ln -s other/logs my_logs $ go build symlink_attack.go $ ./symlink_attack $ cat other/logs - My logs for this process $
As you can see, the content of other/logs is wiped even though our program only interacted with my_logs.
Even in this accidental scenario, os.Create removes important data through its truncation behavior. In malicious scenarios, an attacker could leverage the truncation behavior against the user to remove specific data—perhaps audit logs that would have revealed their presence on the box at one point.
Simple fix, two approaches
To remedy this, we have to insert an os.IsNotExist check before calling Create. If you run the edited symlink_attack.go below, the data in other/logs remains and is not overwritten.
package mainimport (
“fmt”
“io/ioutil”
“log”
“os”
)func main() {
if fileExists("my_logs") { log.Fatalf("symlink attack failure") } file, err := os.Create("my_logs") if err != nil { fmt.Printf("Error creating file: %s", err) } _, err = file.Write([]byte("My logs for this process")) if err != nil { fmt.Printf("Failure to write: %s", err) }}
func fileExists(filename string) bool {
info, err := os.Stat(filename)
if os.IsNotExist(err) {
return false
}
return !info.IsDir()
}
symlink_attack.go with IsNotExist check.
The stipulation here is that by checking os.IsNotExist before creating, we put ourselves in a position where we can’t verify whether a symlink was created between the existence check and the file creation (a time-of-check vs. time-of-use bug). To account for that, we can take a few different approaches.
The first approach is to recreate the implementation of os.Create with your own OpenFile command, thus eliminating the truncation.
func Create(name string) (*File, error) {
return OpenFile(name, O_RDWR\|O_CREATE\|O_TRUNC, 0666)
}
os pkg definition for Create.
package mainimport (
“fmt”
“io/ioutil”
“log”
“os”
“syscall”
)func main() {
file, err := os.OpenFile(“my_logs”, os.O_RDWR|os.O_CREATE|syscall.O_NOFOLLOW, 0666)
if err != nil {
log.Fatal(err)
}_, err = file.Write([]byte("Is this the only thing in the file\n")) if err != nil { fmt.Printf("Failure to write: %s", err) } err = file.Close() if err != nil { fmt.Printf("Couldn't close file: %s", err) } buf, err := ioutil.ReadFile("./my_logs") if err != nil { fmt.Printf("Failed to read file: %s", err) } fmt.Printf("%s", buf)}
symlink_attack.go OpenFile with O_NOFOLLOW to avoid following symlinks.
By opening the file with O_NOFOLLOW, you will not follow a symlink. So when a new file is created, this will work the same as os.Create. However, it will fail to open if a symlink is set up in that location.
The alternative is to create a TempFile and use os.Rename to move it to your preferred location.
package mainimport (
“fmt”
“io/ioutil”
“log”
“os”
)func main() {
tmpfile, err := ioutil.TempFile(".", “”)
if err != nil {
log.Fatal(err)
}os.Rename(tmpfile.Name(), "my_logs") if _, err := tmpfile.Write([]byte("Is this the only thing in the file")); err != nil { log.Fatal(err) } buf, err := ioutil.ReadFile("./my_logs") if err != nil { fmt.Printf("Failed to read file: %s", err) } fmt.Printf("%s", buf)}
symlink_attack.go with TempFile creation and os.Rename.
This pattern broke the symlink between my_logs and other/logs. other/logs still had its contents and my_logs had only the contents “Is this the only thing in the file,” as intended.
Protect future you, now
No matter how careful you are about checking errors in Go, they’re not always behaving the way you might think (tl;dr: rtfm). But updating your practices within Go file creation is really simple, and can save you from unintended consequences.
Article Link: https://blog.trailofbits.com/2020/11/24/smart-and-simple-ways-to-prevent-symlink-attacks-in-go/