Skip to content

Commit

Permalink
Merge pull request #431 from StephenButtolph/gosec
Browse files Browse the repository at this point in the history
handled gosec warnings
  • Loading branch information
StephenButtolph authored Sep 18, 2020
2 parents 71ff97e + ff51276 commit 1180283
Show file tree
Hide file tree
Showing 19 changed files with 35 additions and 28 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ linters:
- goimports
- bodyclose
- govet
- gosec
# - deadcode
# - errcheck
# - varcheck
Expand All @@ -19,7 +20,6 @@ linters:
# - golint
# - gomnd
# - goprintffuncname
# - gosec
# - ineffassign
# - interfacer
# - lll
Expand Down
2 changes: 1 addition & 1 deletion api/admin/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,5 +134,5 @@ func (service *Admin) Stacktrace(_ *http.Request, _ *struct{}, reply *api.Succes

reply.Success = true
stacktrace := []byte(logging.Stacktrace{Global: true}.String())
return ioutil.WriteFile(stacktraceFile, stacktrace, 0644)
return ioutil.WriteFile(stacktraceFile, stacktrace, 0600)
}
6 changes: 3 additions & 3 deletions api/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func TestChangePassword(t *testing.T) {
Password: hashedPassword,
}

password2 := "fejhkefjhefjhefhje"
password2 := "fejhkefjhefjhefhje" // #nosec G101
if err := auth.changePassword("", password2); err == nil {
t.Fatal("should have failed because old password is wrong")
} else if err := auth.changePassword("notThePassword", password2); err == nil {
Expand All @@ -132,7 +132,7 @@ func TestChangePassword(t *testing.T) {
t.Fatal("password should have been changed")
}

password3 := "ufwhwohwfohawfhwdwd"
password3 := "ufwhwohwfohawfhwdwd" // #nosec G101
if err := auth.changePassword(testPassword, password3); err == nil {
t.Fatal("should have failed because old password is wrong")
} else if err := auth.changePassword(password2, password3); err != nil {
Expand All @@ -157,7 +157,7 @@ func TestGetToken(t *testing.T) {
t.Fatal("should have failed because auth token invalid")
}

wellFormedToken := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJFbmRwb2ludHMiOlsiKiJdLCJleHAiOjE1OTM0NzU4OTR9.Cqo7TraN_CFN13q3ae4GRJCMgd8ZOlQwBzyC29M6Aps"
wellFormedToken := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJFbmRwb2ludHMiOlsiKiJdLCJleHAiOjE1OTM0NzU4OTR9.Cqo7TraN_CFN13q3ae4GRJCMgd8ZOlQwBzyC29M6Aps" // #nosec G101
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", wellFormedToken))
if token, err := getToken(req); err != nil {
t.Fatal("should have been able to parse valid header")
Expand Down
4 changes: 2 additions & 2 deletions api/keystore/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
var (
// strongPassword defines a password used for the following tests that
// scores high enough to pass the password strength scoring system
strongPassword = "N_+=_jJ;^(<;{4,:*m6CET}'&N;83FYK.wtNpwp-Jt"
strongPassword = "N_+=_jJ;^(<;{4,:*m6CET}'&N;83FYK.wtNpwp-Jt" // #nosec G101
)

func TestServiceListNoUsers(t *testing.T) {
Expand Down Expand Up @@ -65,7 +65,7 @@ func TestServiceCreateUser(t *testing.T) {
// genStr returns a string of given length
func genStr(n int) string {
b := make([]byte, n)
rand.Read(b)
rand.Read(b) // #nosec G404
return fmt.Sprintf("%x", b)[:n]
}

Expand Down
2 changes: 1 addition & 1 deletion ids/bits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestEqualSubsetBadMiddle(t *testing.T) {

func TestEqualSubsetAll3Bytes(t *testing.T) {
rand.Seed(time.Now().UnixNano())
seed := uint64(rand.Int63())
seed := uint64(rand.Int63()) // #nosec G404
id1 := NewID([32]byte{}).Prefix(seed)
bytes1 := id1.Bytes()

Expand Down
5 changes: 3 additions & 2 deletions network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ func NewNetwork(
pingPongTimeout time.Duration,
pingFrequency time.Duration,
) Network {
// #nosec G404
netw := &network{
log: log,
id: id,
Expand Down Expand Up @@ -845,10 +846,10 @@ func (n *network) connectTo(ip utils.IPDesc) {
// Randomization is only performed here to distribute reconnection
// attempts to a node that previously shut down. This doesn't require
// cryptographically secure random number generation.
delay = time.Duration(float64(delay) * (1 + rand.Float64()))
delay = time.Duration(float64(delay) * (1 + rand.Float64())) // #nosec G404
if delay > n.maxReconnectDelay {
// set the timeout to [.75, 1) * maxReconnectDelay
delay = time.Duration(float64(n.maxReconnectDelay) * (3 + rand.Float64()) / 4)
delay = time.Duration(float64(n.maxReconnectDelay) * (3 + rand.Float64()) / 4) // #nosec G404
}

n.stateLock.Lock()
Expand Down
4 changes: 3 additions & 1 deletion node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,16 @@ func (n *Node) initNetworking() error {
return err
}

// #nosec G402
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
ClientAuth: tls.RequireAnyClientCert,
// We do not use TLS's CA functionality to authenticate a hostname.
// We only require an authenticated channel based on the peer's
// public key. Therefore, we can safely skip CA verification.
//
// TODO: Security audit required
// During our security audit by Quantstamp, this was investigated
// and determinted to be safe and correct.
InsecureSkipVerify: true,
}

Expand Down
2 changes: 1 addition & 1 deletion snow/consensus/snowball/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (n *Network) Finalized() bool {

func (n *Network) Round() {
if len(n.running) > 0 {
runningInd := rand.Intn(len(n.running))
runningInd := rand.Intn(len(n.running)) // #nosec G404
running := n.running[runningInd]

s := sampler.NewUniform()
Expand Down
6 changes: 4 additions & 2 deletions snow/consensus/snowman/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func (n *Network) shuffleColors() {

func (n *Network) Initialize(params snowball.Parameters, numColors int) {
n.params = params
// #nosec G404
n.colors = append(n.colors, &TestBlock{
TestDecidable: choices.TestDecidable{
IDV: ids.Empty.Prefix(uint64(rand.Int63())),
Expand All @@ -43,7 +44,8 @@ func (n *Network) Initialize(params snowball.Parameters, numColors int) {
})

for i := 1; i < numColors; i++ {
dependency := n.colors[rand.Intn(len(n.colors))]
dependency := n.colors[rand.Intn(len(n.colors))] // #nosec G404
// #nosec G404
n.colors = append(n.colors, &TestBlock{
TestDecidable: choices.TestDecidable{
IDV: ids.Empty.Prefix(uint64(rand.Int63())),
Expand Down Expand Up @@ -86,7 +88,7 @@ func (n *Network) Finalized() bool { return len(n.running) == 0 }

func (n *Network) Round() {
if len(n.running) > 0 {
runningInd := rand.Intn(len(n.running))
runningInd := rand.Intn(len(n.running)) // #nosec G404
running := n.running[runningInd]

s := sampler.NewUniform()
Expand Down
2 changes: 1 addition & 1 deletion snow/consensus/snowstorm/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func (n *Network) Finalized() bool {

func (n *Network) Round() {
if len(n.running) > 0 {
runningInd := rand.Intn(len(n.running))
runningInd := rand.Intn(len(n.running)) // #nosec G404
running := n.running[runningInd]

s := sampler.NewUniform()
Expand Down
8 changes: 4 additions & 4 deletions snow/networking/sender/sender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,14 @@ func TestReliableMessages(t *testing.T) {
vdrIDs.Add(ids.NewShortID([20]byte{1}))

sender.PullQuery(vdrIDs, uint32(i), ids.Empty)
time.Sleep(time.Duration(rand.Float64() * float64(time.Microsecond)))
time.Sleep(time.Duration(rand.Float64() * float64(time.Microsecond))) // #nosec G404
}
}()

go func() {
for {
chainRouter.Gossip()
time.Sleep(time.Duration(rand.Float64() * float64(time.Microsecond)))
time.Sleep(time.Duration(rand.Float64() * float64(time.Microsecond))) // #nosec G404
}
}()

Expand Down Expand Up @@ -235,14 +235,14 @@ func TestReliableMessagesToMyself(t *testing.T) {
vdrIDs.Add(engine.Context().NodeID)

sender.PullQuery(vdrIDs, uint32(i), ids.Empty)
time.Sleep(time.Duration(rand.Float64() * float64(time.Microsecond)))
time.Sleep(time.Duration(rand.Float64() * float64(time.Microsecond))) // #nosec G404
}
}()

go func() {
for {
chainRouter.Gossip()
time.Sleep(time.Duration(rand.Float64() * float64(time.Microsecond)))
time.Sleep(time.Duration(rand.Float64() * float64(time.Microsecond))) // #nosec G404
}
}()

Expand Down
2 changes: 1 addition & 1 deletion utils/sampler/uniform_replacer.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (s *uniformReplacer) Sample(count int) ([]uint64, error) {
for i := 0; i < count; i++ {
// We don't use a cryptographically secure source of randomness here, as
// there's no need to ensure a truly random sampling.
draw := uint64(rand.Int63n(int64(s.length-uint64(i)))) + uint64(i)
draw := uint64(rand.Int63n(int64(s.length-uint64(i)))) + uint64(i) // #nosec G404

ret := drawn.get(draw, draw)
drawn[draw] = drawn.get(uint64(i), uint64(i))
Expand Down
2 changes: 1 addition & 1 deletion utils/sampler/uniform_resample.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (s *uniformResample) Sample(count int) ([]uint64, error) {
for i := 0; i < count; {
// We don't use a cryptographically secure source of randomness here, as
// there's no need to ensure a truly random sampling.
draw := uint64(rand.Int63n(int64(s.length)))
draw := uint64(rand.Int63n(int64(s.length))) // #nosec G404
if _, ok := drawn[draw]; ok {
continue
}
Expand Down
4 changes: 2 additions & 2 deletions utils/sampler/weighted_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func WeightedPowBenchmarkSampler(

b.ResetTimer()
for i := 0; i < b.N; i++ {
_, _ = s.Sample(uint64(rand.Int63n(int64(totalWeight))))
_, _ = s.Sample(uint64(rand.Int63n(int64(totalWeight)))) // #nosec G404
}
return true
}
Expand All @@ -137,7 +137,7 @@ func WeightedSingletonBenchmarkSampler(b *testing.B, s Weighted, size int) bool

b.ResetTimer()
for i := 0; i < b.N; i++ {
_, _ = s.Sample(uint64(rand.Int63n(math.MaxInt64)))
_, _ = s.Sample(uint64(rand.Int63n(math.MaxInt64))) // #nosec G404
}
return true
}
Expand Down
2 changes: 1 addition & 1 deletion utils/sampler/weighted_best.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (s *weightedBest) Initialize(weights []uint64) error {
// here, as the generated numbers are only used to perform an
// optimistic benchmark. Which means the results could be arbitrary
// and the correctness of the implementation wouldn't be effected.
samples[i] = uint64(rand.Int63n(int64(totalWeight)))
samples[i] = uint64(rand.Int63n(int64(totalWeight))) // #nosec G404
}
}

Expand Down
5 changes: 3 additions & 2 deletions utils/sorting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ func TestSort2dByteArray(t *testing.T) {
// Create a random 2D array
arr := [][]byte{}
for i := 0; i < numSubArrs; i++ {
subArrLen := rand.Intn(maxLength)
subArrLen := rand.Intn(maxLength) // #nosec G404
subArr := make([]byte, subArrLen)
if _, err := rand.Read(subArr); err != nil {
_, err := rand.Read(subArr) // #nosec G404
if err != nil {
t.Fatal(err)
}
arr = append(arr, subArr)
Expand Down
2 changes: 1 addition & 1 deletion vms/avm/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func sampleAddrs(t *testing.T, vm *VM, addrs []ids.ShortID) ([]ids.ShortID, []st
t.Fatal(err)
}

numAddrs := 1 + rand.Intn(len(addrs))
numAddrs := 1 + rand.Intn(len(addrs)) // #nosec G404
for i := 0; i < numAddrs; i++ {
indices, err := sampler.Sample(1)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion vms/avm/vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ var addrs []ids.ShortID // addrs[i] corresponds to keys[i]

var assetID = ids.NewID([32]byte{1, 2, 3})
var username = "bobby"
var password = "StrnasfqewiurPasswdn56d"
var password = "StrnasfqewiurPasswdn56d" // #nosec G101

func init() {
cb58 := formatting.CB58{}
Expand Down
1 change: 1 addition & 0 deletions vms/platformvm/import_tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func TestNewImportTx(t *testing.T) {
sm := m.NewSharedMemory(vm.Ctx.ChainID)
peerSharedMemory := m.NewSharedMemory(avmID)

// #nosec G404
utxo := &avax.UTXO{
UTXOID: avax.UTXOID{
TxID: ids.GenerateTestID(),
Expand Down

0 comments on commit 1180283

Please sign in to comment.