Wire: Expression 2 applyForce server crash exploit/bug

[ghost]

Video:
https://www.youtube.com/watch?v=F7qQWT7dKGU

Code:

@name Rekt
if (first()) {
    runOnTick(1)
}
if (duped()) {
    selfDestruct()
}
entity():applyForce(vec(sinr(curtime() * 1e24) * 1e+24,cosr(curtime() * 1e24) * 1e+24,cosr(curtime() * 1e24) * 1e+24))

Unfreeze it after you spawn it and server should crash.

[oldmud0]

but why

also that's probably not our fault. Test it in raw Lua.

[CaptainPRICE]

Duplicate of #1188.

@theplayer232
Perhaps code just looks different to you, but it is the same function which causes it to crash. So, no need for two same issues.
Learn how to use Markdown.

@oldmud0

Test it in raw Lua.

How, lol. You mean GLua.
Yep, tested and it crashed the server as expected:

if not SERVER or #player.GetAll() < 1 then return end

local EntID = 0  -- ID of entity to use.

local ent = NULL
if EntID > 0 then
    ent = Entity(EntID)
else
    local ply = player.GetAll()[1]
    if IsValid(ply) then
        local tr = ply:GetEyeTrace()
        ent = ents.Create("prop_physics")
        if IsValid(ent) then
            ent:SetModel("models/props_c17/canister02a.mdl")
            ent:SetPos(tr.HitPos + ply:GetAimVector() * -16)
            ent:Spawn()
        else
            ent = tr.Entity
        end
    end
end
if IsValid(ent) then
    local phys = ent:GetPhysicsObject()
    if IsValid(phys) then
        print("Crashing . . .", ent, phys)
        phys:EnableMotion(true)  -- Unfreeze.
        phys:ApplyForceCenter(Vector(1e54, 1e54, 1e54))  -- Apply abnormal force.
    else
        print("Pick entity with valid physics object", EntID)
    end
else
    print("Pick valid entity", EntID)
end

This should not be difficult to fix, simply clamp the vector should fix it, right?
And then just add ConVar to make it easy to control the behavior.

[ghost]

@CaptainPRICE Just wanted to mention on the reference you made to this issue report is that this doesn't actually require PropCore to be enabled to work, the other issue instead needs PropCore.

[CaptainPRICE]

Yeah, #1188 requires propcore just so it can spawn a prop. But, that code is not necessary to reproduce the issue, that means that part of code is unrelated as it is not required.
And here you seem you have minimized it. So, I'll go ahead and minimize it further:

interval(1)
entity():applyForce(vec(1e54))

See, that's minimized code, which is just enough to make it crash. No propcore.
Another words, propcore has nothing to do with this. so why mention/use it.
Now, with minimized code we can clearly see what it does. :)

[oldmud0]

So clearly 1e54 is the magic value to kill the physics engine. What is the crash dump/engine error message?

[ghost]

Values that have worked: 1e+24 and 1e24
those two are the ones that causes extremely high number.

[bigdogmat]

Those represent the same number, the question here is what should we clamp the vectors to

[oldmud0]

The question is, should _we_ be making the workarounds here? I do believe it's better to let the GMod developers know about this problem so that it can be resolved at the root. Also, what's stopping players from just dividing the force into two applyForce commands?

[shadowscion]

@oldmud0 I don't think wiremod should 'fix' this, it shouldn't really be possible to crash something bearing the name 'sandbox'; report it on the garrysmod github.

[CaptainPRICE]

@bigdogmat

the question here is what should we clamp the vectors to

I've just wrote an automatic E2 to figure that:

# To change the model of E2, uncomment a line below if you wish. LED is the smallest model I know of, however I haven't noticed any changes, like "if using different model it wouldn't not crash", so that's nothing to do with crash issue.
#@model models/led.mdl

# We want these as outputs, right, so we can see them via Debugger (or whatever).
@outputs ExpressionGate:entity Busy E N:string F:vector

# Since there are no inputs, I'll just set trigger to none.
@trigger none

# I call this an "Automation pattern" (with timer)..  ;-)
if (clk("beginApplyForce") | clk("endApplyForce") | clk("prepareApplyForce") | clk("increment"))
{
    (clkName())()
}
elseif (first())
{
    ExpressionGate = entity()
    ExpressionGate:setMass(1)  # That's a kilogram, right? Still crash.
    E = +35  # From which exponent to start from. Note: E is incremented by default.

    function endApplyForce()
    {
        if (!Busy) { stoptimer(clkName()), return }
        print("End of " + E)
        ExpressionGate:propFreeze(1)  # Freeze physics object.
        ExpressionGate:setAng(ang())  # Zero angles.
        ExpressionGate:setPos(vec())  # Move it to origin.
        timer("increment", 1000)
        Busy = 0
    } # End of function void=endApplyForce()

    function beginApplyForce()
    {
        if (!Busy) { stoptimer(clkName()), return }
        print("Begin of " + E)
        ExpressionGate:applyForce(F)
        timer("endApplyForce", 1000)
    } # End of function void=beginApplyForce()

    function prepareApplyForce()
    {
        if (!Busy) { stoptimer(clkName()), return }
        print("Preparing for " + E)
        ExpressionGate:setAng(ang())  # Zero angles.
        ExpressionGate:setPos(vec())  # Move it to origin.
        ExpressionGate:propFreeze(0)  # Unfreeze physics object.
        timer("beginApplyForce", 1000)
    } # End of function void=prepareApplyForce()

    function increment()
    {
        if (Busy) { return }
        Busy = 1
        E++  # Direction of force. Use E++ (to post-increment E by 1) or E-- (post-decrements E by 1).
        N = "1e" + (sign(E) == -1 ? "-0" : "+0") + E  # Construct an exponential notation string, E being the exponent.
        F = vec(N:toNumber(10))  # Parse N as decimal number with base 10, and construct a 3D vector.
        timer("prepareApplyForce", 1000)
    } # End of function void=increment()

    increment()  # Start the "increment" loop initially.
} # End of first()

That was pretty easy to do. Here are the results:

...
Preparing for 38
Begin of 38
End of 38
Preparing for 39

Stopped at 39 as that's where it crashed. So, I guess it should also then crash at -39, too (just replace E++ with E-- on line 54, and run it).
So, now we know that.. Here is my set of temporary E2 functions, if anybody will find them useful (though I don't know why would you even..):

if (first())
{
#ifdef entity:applyForce(vector)
    function safeApplyForce(Force:vector)
    {
        entity():applyForce(clamp(Force, 1e-38, 1e+38))
    } # End of function void=safeApplyForce(v)

    function entity:safeApplyForce(Force:vector)
    {
        This:applyForce(clamp(Force, 1e-38, 1e+38))
    } # End of function void=e:safeApplyForce(v)
#endif

#ifdef entity:applyOffsetForce(vector, vector)
    function safeApplyOffsetForce(Force:vector, Position:vector)
    {
        entity():applyOffsetForce(clamp(Force, 1e-38, 1e+38), clamp(Position, 1e-38, 1e+38))
    } # End of function void=safeApplyOffsetForce(v,v)

    function entity:safeApplyOffsetForce(Force:vector, Position:vector)
    {
        This:applyOffsetForce(clamp(Force, 1e-38, 1e+38), clamp(Position, 1e-38, 1e+38))
    } # End of function void=e:safeApplyOffsetForce(v,v)
#endif
} # End of first()

# Usage:
entity():propFreeze(0)  # Unfreeze E2.
## Center:
safeApplyForce(vec(1e+54))  # Now crazy physics should not crash a server.
safeApplyForce(vec(1e-54))  # It should not crash in another direction either.
## Offset:
safeApplyOffsetForce(vec(1e+54), vec())
safeApplyOffsetForce(vec(1e-54), vec())

print("Success! You have not crashed the server by using applyForce in E2.\n" + owner():name() + " has unlocked a new achievement 'Truth Server Physicist'.")
entity():setPos(owner():aimPos())  # Move E2 where owner aims at.

:beers:

@oldmud0 Yeah, I agree that Wiremod should not 'fix' this, instead should be reported here (or PM Rubat).. @shadowscion, as you can see issues are no longer there on garrysmod repo.
However, above is just a temporary E2 solution to keep server from crashing when working with huge applyForce (until it becomes fixed).. Clamping arguments between [1e-38, 1e+38] of applyForce and applyOffsetForce functions seems to be enough, at least that's what above E2 does.
So, if you attempt to apply abnormal force, clamp function will take care of that, meaning a vector you specify will always be within a maximum range which does not lead a server to crash (at least for me, on Windows).

[CaptainPRICE]

Hah! Nailed it, any Vector whose component reaches 1e+039 will have magnitude/length equal to INF:

Now to simplify my comment above..

[Divran]

I knew there had to be an infinite value somewhere.

Edit: or at least math.huge

[shadowscion]

@CaptainPRICE https://github.com/Facepunch/garrysmod-issues issues are and always have been there

[TomyLobo]

could it be that gmod casts that stuff from Lua numbers (which are double-precision floats) to C++ floats (i.e. single precision) somewhere?

[TomyLobo]

Nailed it:
Float's non-zero absolute values range from 1.8E-38 to 3.4E+38

See also: http://ideone.com/GOhGrQ

[Divran]

Alright so we can run the input to all applyforce functions through a different version of E2Lib.clampPos and make that function clamp it between 1.8E-38 and 3.4E+38.

EDIT: what to call it? clampForce?
EDIT: Nevermind. Read my next post below.

[CaptainPRICE]

could it be that gmod casts that stuff from Lua numbers (which are double-precision floats) to C++ floats (i.e. single precision) somewhere?

Yep, that would make sense as vec_t is a float..
https://github.com/ValveSoftware/source-sdk-2013/blob/master/mp/src/public/mathlib/vector.h#L70

[Divran]

All of the applyforce functions already use a function called check. This function checks for math.huge. If the force vector is larger than math.huge, it doesn't apply the force at all. What we're proposing here is a choice between these two behaviors:

• To not apply any force at all if it's larger than 3.4E+38 or smaller than 1.8E-38
• Or to clamp it to those values instead, so that if you give it a value larger/smaller than these numbers, it'll still apply a force, just not any larger/smaller than that.

For now, I'll update the code to not apply any force at all, but I'll not commit/push it yet, to give everyone a chance to respond to this.

@CaptainPRICE you've been testing this crash for a while now. Could you test this real quick? Just open core/entity.lua and replace lines 445-457 with this

local min_force = 1.8E-38
local max_force = 3.4E+38
local function check( v )
    return  min_force < v[1] and v[1] < max_force and
            min_force < v[2] and v[2] < max_force and
            min_force < v[3] and v[3] < max_force
end

local function checkv( v )
    return  min_force < v.x and v.x < max_force and
            min_force < v.y and v.y < max_force and
            min_force < v.z and v.z < max_force
end

EDIT: And I totally forgot that we have several other applyForce functions in other files which have been completely neglected over the years. I'm going to move the check function to E2Lib so that all files have access to it. @CaptainPRICE in the mean time you could check the above code

[CaptainPRICE]

@TomyLobo Is somewhat wrong about the range ;)
I know for a fact this is the correct range for a single-precision floating-point number:
-3.402823E+38 to 3.402823E+38
And for double-precision floating-point number:
-1.79769313486232E+308 to 1.79769313486232E+308

Ah, yeah I gotta fix that range in #1190 ..

@Divran k. I'm on it...
EDIT: Okay, so I did that on master branch:

local min_force = -3.402823e+38
local max_force =  3.402823e+38

local function check(v)
    return  -min_force < v[1] and v[1] < max_force and
            -min_force < v[2] and v[2] < max_force and
            -min_force < v[3] and v[3] < max_force
end

local function checkv(v)
    return  -min_force < v.x and v.x < max_force and
            -min_force < v.y and v.y < max_force and
            -min_force < v.z and v.z < max_force
end

That didn't crash with huge numbers, but as you just said, other functions doesn't have this check (which I have noticed in #1190). So, what to do now?
You move these two into E2Lib as checkForce and checkForceVector?
About what behavior to use:
I compared vector's length/magnitude in #1190, and that in turn wouldn't do anything if you would give it 1e+54 for example (in any component X/Y/Z). I would prefer to keep it like that, because that's how it "_partially works_" right now, it is just that range in there is wrong (math.huge instead of float min and max) and other functions neglected that check :)
Although, clamping is not a bad choice, but it is somewhat overhead maybe?

EDIT 2: Don't forget Wire Gates, apply force, apply offset force.. ;)
Oh, I see I screwed it up in #1190 as I thought that length/magnitude check would be better than "magic numbers". Oh well..

[Divran]

@CaptainPrice alright try editing the code to 3.4E-38 and see if it works

EDIT: I just realized there's an extra minus sign in my code. I edited the above code to fix it.

[Divran]

I ended up adding the check to WireLib instead of E2Lib because @CaptainPRICE reminded me of the existence of the applyForce gates

[TomyLobo]

@CaptainPRICE stop being a smartass, of course it's not a round number, considering we're talking decimal representations for numbers stored in binary format here.
And you did notice I said non-zero absolute values, right?
Absolute values don't become negative if you didn't know.
Words, they mean stuff.

[oldmud0]

Yeah but how many other things don't have clamps? It's not just force that can crash the game you know. Tons of other stuff that uses floats.

[CaptainPRICE]

@TomyLobo

considering we're talking decimal representations for numbers stored in binary format here.

What binary format are you talking about?! (.. float size is (always) 32 bits in (gmod) memory ..)
Decimal digits are also very important as it makes a BIG difference. FLOAT_MAX != 3.4E+38

And you did notice I said non-zero absolute values, right?

Yes.. Look, I have wanted to correct you and I did. Because, both of the values you have posted are completely wrong:

Float's non-zero absolute values range from 1.8E-38 to 3.4E+38

Also if you haven't noticed it, it was completely unnecessary to post "non-zero absolute minimum value", by the way there is one word for that and I think it is called Epsilon, now I've teach you that. Moving onto the actual thing.. See these two screenshots below:

See? You have been corrected, twice! Accept it, and be happy.
Okay, I am going to be even more precise now, so here they are - the correct (constant) values:

FLOAT_EPSILON     = 1.4e-45            -- The smallest positive float value that is greater than zero.
FLOAT_EPSILON_ARM = 1.175494351e-38    -- The smallest positive float value that is greater than zero (use only on ARM-processor systems, as FLOAT_EPSILON is too small to be detected, so it equates to zero).
FLOAT_MAX  =  3.40282346638528859e+38  -- The largest possible value of float.
FLOAT_MIN  = -3.40282346638528859e+38  -- The smallest possible value of float.

You wanted magic numbers, you got them now. I guess that is okay, since that is the only way ;)
Now watch this outcome. Because you have provided incorrect range value ("Max: 3.4E+38"), @Divran have used that wrong (_non-precise_) information in f8e3e74aa4a6f52b204a4ac6c11f564048a5c014 commit. Now we can't do applyForce(vec(3.4028234e+38)), which is valid force (would not cause a server to crash)..

Absolute values don't become negative if you didn't know.

... by looking at your comment, I was like "What crap is this guy spewing?! When I know those values on top of my head, (_LOL_), the values he has posted are not correct" ...

stop being a smartass
[...]
Words, they mean stuff.

You have got my upvote because you have a good point, but also a downvote because of incorrect values.
Please, don't be jelly my friend, sorry I just know better than you. Now, calm down man. k? :sunglasses:

@oldmud0

It's not just force that can crash the game you know. Tons of other stuff that uses floats.

Yep. Sure it is not just apply force.. As nobody was apparently aware of this type-casting from another side, until now, that is just ridiculousness after so many years..

[oldmud0]

I just know better than you

hey leave the ego in the forums will you

[AbigailBuccaneer]

@CaptainPRICE, consider the ways in which you're acting aren't helping the situation right now. I'm locking this conversation thread.

[TomyLobo]

Since @AbigailBuccaneer closed the thread, let me just make a few corrections to what you said.

1. GMod doesn't run on ARM, so mentioning ARM is unnecessary, but apart from that, you're also wrong:
2. GMod Lua numbers are doubles, which are 64 bits wide.
3. Epsilon is not what I was talking about. IEEE doesn't even define epsilon. In broader terms, Machine epsilon is a relative error, which has nothing to do with the minimum absolute value for any floating point type used in GMod.

I had to whip up a VM to test point 1 and I had to look up point 3, but I'd rather know what I don't know than to claim bullshit as the truth and feel superior to other people when I am clearly wrong.

_I_ was wrong in one point, too, though.
http://ideone.com/vXAWp8
I forgot about denormal numbers. For standard single-precision floating point numbers, that's everything between roughly 7.1e-46 (=0.71e-45 i.e. about half your FLOAT_EPSILON) and 1.18e-38 (which you for some reason call FLOAT_EPSILON_ARM).

[Divran]

Well, @TomyLobo I'm not afraid to admit I have no clue what the correct values are so feel free to edit the values in the check function if they are wrong. The current values seemed to work in-game though.

[TomyLobo]

They don't really matter anyway. No one is going to try any values that high anyway.
The +/-3.4e38 you picked is fine