INPStarfall / Starfall

Starfall Processor for Garry's Mod + Wiremod
http://www.wiremod.com/forum/developers-showcase/22739-starfall-processor.html
Other
17 stars 15 forks source link

Vulnerability fix for CheckType #406

Closed Jazzelhawk closed 8 years ago

awilliamson commented 8 years ago

@Jazzelhawk Could you attribute the finder of the bug in the commit message please. Other than that, makes sense. Just discussing things with @Xandaros about safety critical use of CheckType and whether what's the blame here. Clearly some design fault, but where to fix it is a question. For now this fix is a great idea.

Jazzelhawk commented 8 years ago

Okay, I amended the commit with credit to KubeRoot. As far as I know he was the one to find the bug.

awilliamson commented 8 years ago

Maybe we should consider a change to how we internally register types. We have SF.Types[ name ] = metamethods. Maybe this could be changed and exploited to fit the ability to tag unique types internally. Rather than metamethods.__real_types? Perhaps by using the metamethods as a key rather than a string name, this could provide easy lookup and validation? Would like @Xandaros thoughts on this, I've not 100% thought this idea through, but will look into it.

Jazzelhawk commented 8 years ago

I just tested changing this line:

if meta == typ or ( meta and meta.__supertypes and meta.__supertypes[ typ ] and meta.__realType == real_type ) then return val end

to this line:

if meta == typ or ( meta and meta.__supertypes and rawget( meta.__supertypes, typ ) and type( typ ) == "table" ) then return val end

And I feel like this would work fine.

awilliamson commented 8 years ago

@Jazzelhawk I'd suggest making a new issue. Something along the lines of fixing the real-type checking system to make use of internal registration via key lookup, via the method I suggested above. When I have some more time available, I'll try looking into it, and see what we can do to fix it up properly. But for now, this addressed the vulnerability fix, albeit in a slightly round-about symptomatic fix kind of way.