WARNING!! Using tonumber on a client-input string is dangerous!

consider the following:

[lua]
concommand.Add(“dropmoney”, function(p,,,s)
local amount = tonumber(s);
if(amount <= 0) then return; end
if(p:GetMoney() < amount) then return; end

p:SetMoney(p:GetMoney() - amount);

CreateMoneyEntity(amount):SetPos(p:GetEyeTrace().HitPos));

end);
[/lua]

This is completely exploitable and can be abused to make your money “NaN”
How?

in console



dropmoney nan


Why would this cause it?

Explanation:
In Lua 5.2 and above they remove a feature, tonumber"nan" and tonumber"inf". These would respectively return the result of 0/0 and math.huge.

NaN (0/0) will return false for EVERY comparison, even against itself…



] m> tostring(0/0), 0/0 == 0/0
"nan"	false	


This is dangerous.

math.huge, also known as tonumber"inf" is also relatively dangerous. Not as much so as nan, but it’s still something you might want to check for.

a function to check if the number is finite:

[lua]
local function IsFinite(num)
return not (num ~= num or num == math.huge or num == -math.huge);
end
[/lua]

using this to fix our code above…

[lua]
concommand.Add(“dropmoney”, function(p,,,s)
local amount = tonumber(s);
if(not IsFinite(amount)) then return; end
if(amount <= 0) then return; end
if(p:GetMoney() < amount) then return; end

p:SetMoney(p:GetMoney() - amount);

CreateMoneyEntity(amount):SetPos(p:GetEyeTrace().HitPos));

end);
[/lua]

I also advise you to NEVER make a string into a number, like concommands or net.WriteString a numeric-string. Always use net.WriteLong, net.WriteInt, net.WriteUInt, net.WriteDouble, etc. when possible!

The thread says it’s solved, but it’s not solved???:scream:

http://i.imgur.com/j7pizZM.png[/t]
[t]http://i.imgur.com/ND2KGJK.png

[editline]now[/editline]

credits: willox

Would it be incredible harmful to just detour tonumber to not accept nan or +/-inf?

Can you please write a short tutorial on how I can exploit this bug to become rich on DarkRP servers? Really want to buy some FNAF skins.

I think that darkrp can check if that number it’s real by:

[lua]if amount >= 2147483647 then[/lua]

I created a pr to make it a standard library function! Please support if you realize the harm this could cause!

Yeah, DarkRP is pretty good about verifying its numbers but unfortunately many DarkRP addons are not.

I didn’t believe this until I went in game and tried it myself. Depending on how you code your checks this could be more or less a non-issue ( money >= amt, always would be false ), but it is interesting.

Although this isn’t quite true in a March 2009 build of Lua 5.1:


print( _VERSION )

print( tostring( tonumber( "nan" ) ) )

amt = tonumber( "nan" )

money = 21

if money >= amt then
	print( "did stuff!" )
	--do stuff
end


Lua 5.1
nil
[string "chunk"]:9: attempt to compare nil with number

Might be a specific branch of 5.1 that has this “feature” , or more related to luajit.

That does not work.
nan >= number always returns false.

Also other fun facts:
nan < number always returns false
nan == number always returns false
nan > number always returns false

and the best
nan == nan returns false.

Don’t forget about -inf

And this returns false too, then this statement it’s correct and safe

[lua](self:getDarkRPVar(“money”) or 0) - math.floor(amount) >= 0[/lua]

Your code is wrong:

[lua]function math.IsFinite( value )
return not ( num ~= num or num == math.huge or num == -math.huge )
end[/lua]

Your function defines “value” but your check uses “num”

yeah i realized before you posted, but thanks anyways

[editline]29th July 2015[/editline]

:suicide:

I have always used this function to check for terrible numbers, it’s pretty solid

Credits to whoever made this, i don’t remember, maybe Python1320 or CapsAdmin from here.


	------------------------
	-- Might be useful
	------------------------
	local inf,ninf,ind = 1/0,-1/0,(1/0)/(1/0)
	--(ind==ind) == false :(. This should do though. >= and <= because you never know :3
	function math.BadNumber(v) 
		return not v or v==inf or v==ninf or not (v>=0 or v<=0)
	end

it goes deeper than we thought

What’s wrong with it? I thought TTT’s intended intended purpose was to be an example of a badly coded gamemode?

1 / 0 == inf and behaves normally.
0 / 0 fucks everything up in every version of Lua.

Try this one in the demo

[lua]
local amount = 0/0
local money = 500

if amount > money then
print(“NOT ENOUGH MONEY 2”)
else
print(“sure :slight_smile: 1”)
end

if amount <= money then
print(“sure :slight_smile: 2”)
else
print(“NOT ENOUGH MONEY 2”)
end

sure :slight_smile: 1
NOT ENOUGH MONEY 2
[/lua]

Whether you check on a condition or its inversion (b > a vs b <= a) makes the difference in behaviour.

Literally, this rule is broken:



∀a, b. a > b ≡ ¬(b ≤ a)


[lua]a = 0/0
b = 20 – any arbitrary number

print(a > b == not (b <= a))
> false
[/lua]

Sadly, this is actually the IEEE 754 standard for NaN numbers and we have no choice but to deal with it.

The question is, should the solution be in tonumber or are we to do a nan check in every function that takes client number input?

Logically, this issue could be fixed by simply having an isnan, could it not? I’m sure in the future it’d be nice to have an isnan for GLua (is there even an implementation of isnan in Lua itself?)

Long story short, can there, and more importantly, will there be an isnan function ever implemented into GMod?

EDIT: Nevermind, that link talks nothing about comparisons between NaN, but we do need something that allows us to check if something is NaN, it seems that comparisons still don’t give a heck though because you can’t actually check if
NaN == NaN, the stacktrace talks about it as being there for multiple reasons, one of which is to keep calculations from continuing forever.

I fixed it on my server by just returning nil if nan or inf is entered (even though inf doesn’t cause any problems). I just don’t really see the benefit of tonumber being able to return NaN…
On the other hand this is pretty much just standard, almost all other languages that allow to parse a string into double can also produce NaN.

Most of the exploitable functions could be avoided if lua actually had strict types and integers… :v:

It’s just great how nan breaks simple logical rules
[LUA]
(money < nan or money > nan) == !(money >= nan and money <= nan) – evaluates to false
[/LUA]
:mindblown:

People are unlikely to know about NaN and use isnan. Changing the behaviour of tonumber is “safer” in the sense that it will immediately fix a lot of potential exploits in a wide range of scripts including ones included in gmod by default.