common/BitArray: Or is buggy with a nil deference and mistaken nil check clause instead of a return #146

odeke-em · 2018-02-06T08:41:24Z

I was just auditing the code and noticed that *BitArray.Or's code looks like this

Lines 102 to 104 in 19e818f

    
           if bA == nil { 
        
           	o.Copy() 
        
           }

Notice the bug there
if bA == nil, we invoke o.Copy(), of which o might also be nil. But the biggest problem even is that even if bA, we intended to return o.Copy() but instead forgot about the return and on the next line invoke bA.mtx.Lock() which delivers a crash to us.

We really need to make rigorous tests as it seems that we've been testing with the best conditions intended but in the wild we find the actual edge cases e.g nil arguments being passed in as in tendermint/tendermint#1169

Fixes #145 Fixes #146 The code in here has been fragile when it comes to nil but these edge cases were never tested, although they've showed up in the wild and were only noticed because the reporter actually read the logs otherwise we'd have never known. This changes covers some of these cases and adds some tests.

odeke-em · 2018-02-08T21:40:06Z

Fixed by #147 which has been merged into develop.

odeke-em changed the title ~~common/BitArray: Or is buggy with a nil deference and mistaken nil check clause~~ common/BitArray: Or is buggy with a nil deference and mistaken nil check clause instead of a return Feb 6, 2018

odeke-em mentioned this issue Feb 6, 2018

common/BitArray: reduce fragility with methods #147

Merged

odeke-em closed this as completed Feb 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

common/BitArray: Or is buggy with a nil deference and mistaken nil check clause instead of a return #146

common/BitArray: Or is buggy with a nil deference and mistaken nil check clause instead of a return #146

odeke-em commented Feb 6, 2018

odeke-em commented Feb 8, 2018

common/BitArray: Or is buggy with a nil deference and mistaken nil check clause instead of a return #146

common/BitArray: Or is buggy with a nil deference and mistaken nil check clause instead of a return #146

Comments

odeke-em commented Feb 6, 2018

odeke-em commented Feb 8, 2018