Audit Unsafe
- Dominant language
- Go
- Stars
- 17.1k
- Forks
- 3.2k
- Avg merge
- 3d 18h
- Merged PRs (30d)
- 11
Description
Given #4483, we need to audit all usages of unsafe. We generally avoid it but some of the libraries we use use it.
# Libraries
## go-os-rename
**UNSAFE**
https://github.com/ipfs/go-ipfs/issues/4808#issuecomment-375813269
~~However, it appears to not handle long paths, unlike the built-in one. Furthermore, it looks like go now replaces existing files on windows (i.e., the reason we use this library no longer applies). We should stop using it.~~
## go-sockaddr
**SAFE?**
Mostly exported from go-ipfs. However, I've asked the reporter in #4483 to try disabling it to see if that fixes it.
## go-peerstream
**SAFE**
Used to compare pointers to avoid a deadlock.
## websocket
**SAFE**
Used to do a fast XOR. Looks safe.
## bbloom
**UNSAFE?**
Used all over the place and go vet complains that it's used incorrectly. I don't believe we use bbloom in any critical places so we can probably switch to bloom (made by the same author without unsafe).
## gogo-protobuf
**UNKNOWN**
Unclear but probably well vetted. I'm in the process of upgrading this library anyways (so if there are any missing bug fixes, that should pull them in).
## fuse
**LOL**
## go-net
**SAFE?**
Made by the go authors. Regardless we should update this.
## fsnotify.v1
**SAFE**
Looks fine.
## sys
**SAFE?**
We should try to upgrade to the latest version everywhere anyways.
## Badger
**UNKNOWN**
Their mmap code looks fine. I'm mostly worried about the skip-list implementation. However, I doubt the user in #4483 has badger enabled.
## go-codec
**UNKNOWN**
## murmur3
**UNKNOWN**
However, we don't use this so it's definitely not causing any issues.
On the other hand, it looks like it may be doing unaligned loads (problem on arm?).
## go-colorable
**SAFE**
Used on windows for syscalls. Definitely safe.
## go-logging
**SAFE?**
The memory backend uses unsafe. However, it looks like it's just using it to atomically swap pointers. From what I can tell, this all looks safe.
## go-crypto
**SAFE?**
Probably safe. Might as well update.
## leveldb
**UNKNOWN**
Uh.... Yeah. This one's going to be hard to audit. We should just update it and hope.
## go-text
**UNLIKELY**
Written by the go authors. However, we should update it.
## pb (progress bar)
**SAFE**
Only uses unsafe for a few simple ioctls. Unlikely to be an issue.
## GoEndian
**SAFE**
Checked.
## hang-fds
**WHO CARES**
For testing, looks fine bug I don't particularly care.
# TODO
* ~[ ] Check if disabling fuse fixes #4483~
* [x] Check if disabling reuseport fixes #4483
* [ ] Switch to a better fuse library.
* [x] Upgrade gogo-protobuf.
* [x] Upgrade go-net.
* [x] Switch to safe bbloom (if fast enough). Otherwise, check bbloom.
* Fixed bbloom
* [ ] Get badger to use a safe bloom filter implementation (https://github.com/dgraph-io/badger/issues/1005)
* [x] Make sure to upgrade everything to the latest sys.
* [x] Check badger's skip-list (and probably give up...).
* This has been audited.
* [x] Update go-codec.
* [x] Update go-crypto.
* [x] Stop using go-os-rename
* [x] Update goleveldb
* [x] Update go-text
Contributor guide
Assessment
This issue has not been assessed yet.