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

[MeepDarknessM]

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!

[Exploderguy]

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

[MeepDarknessM]

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

[editline]now[/editline]

credits: willox

[cogg]

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

[SirSavary]

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.

[G😹nz😻]

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

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

[MeepDarknessM]

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

[url]https://github.com/garrynewman/garrysmod/pull/1032[/url]

[UselessGhost]

[QUOTE=gonzalolog;48326221]I think that darkrp can check if that number it's real by:

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

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

[Kogitsune]

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.

[quote]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. [/quote]

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.

[Fredy]

[QUOTE=Joeyl10;48326269]Yeah, DarkRP is pretty good about verifying its numbers but unfortunately many DarkRP addons are not.[/QUOTE]

That does not work.
nan >= number always returns false.
[url]http://puu.sh/ji3HI/833fc265b8.png[/url]
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.

[QUOTE=MeepDarknessM;48326235]I created a pr to make it a standard library function! Please support if you realize the harm this could cause!

[url]https://github.com/garrynewman/garrysmod/pull/1032[/url][/QUOTE]

Don't forget about -inf

[G😹nz😻]

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

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

[HighVoltage]

[QUOTE=MeepDarknessM;48326235]I created a pr to make it a standard library function! Please support if you realize the harm this could cause!

[URL]https://github.com/garrynewman/garrysmod/pull/1032[/URL][/QUOTE]

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"

[MeepDarknessM]

[QUOTE=syl0r;48326284]
Don't forget about -inf[/QUOTE]

yeah i realized before you posted, but thanks anyways

[editline]29th July 2015[/editline]

[QUOTE=highvoltage;48326317]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"[/QUOTE]

:suicide:

[Nafrayu]

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

[MeepDarknessM]

[url=https://github.com/garrynewman/garrysmod/blob/ead2b4d7f05681e577935fef657d4a4d962091e6/garrysmod/gamemodes/terrortown/gamemode/weaponry.lua#L445-L472]it goes deeper than we thought[/url]

[aStonedPenguin]

[QUOTE=MeepDarknessM;48326851][url=https://github.com/garrynewman/garrysmod/blob/ead2b4d7f05681e577935fef657d4a4d962091e6/garrysmod/gamemodes/terrortown/gamemode/weaponry.lua#L445-L472]it goes deeper than we thought[/url][/QUOTE]

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

[FPtje]

[QUOTE=DrogenViech;48326624]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

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

Try this one in the [url=http://www.lua.org/cgi-bin/demo]demo[/url]

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

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

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

> sure :) 1
> NOT ENOUGH MONEY 2
[/lua]

Whether you check on a condition or its inversion ([U]b > a[/U] vs [U]b <= a[/U]) 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, [url=http://stackoverflow.com/questions/1565164/what-is-the-rationale-for-all-comparisons-returning-false-for-ieee754-nan-values]this is actually the IEEE 754 standard for NaN numbers[/url] 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?

[AlvyP]

[QUOTE=FPtje;48329956]Words[/QUOTE]

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?
[url]http://lua-users.org/wiki/InfAndNanComparisons[/url]

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.

[Fredy]

[QUOTE=FPtje;48329956]
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?[/QUOTE]
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:

[QUOTE=FPtje;48329956]
Whether you check on a condition or its inversion ([U]b > a[/U] vs [U]b <= a[/U]) makes the difference in behaviour. [/QUOTE]
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:

[FPtje]

[QUOTE=Reyjr43;48330064]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?)[/QUOTE]

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.

[MeepDarknessM]

[QUOTE=syl0r;48330103]I fixed it on my server by just returning nil if nan or inf is entered (even though inf doesn't cause any problems). [/QUOTE]

don't forget about -inf

[Mijyuoon]

[QUOTE=MeepDarknessM;48331358]don't forget about -inf[/QUOTE]

What's with it? It's just smaller than any other number. Can be protected against by simple "a < 0" check that you most likely already have. Even without a special case in tonumber fixer it most likely won't cause problems.

[MeepDarknessM]

[QUOTE=mijyuoon;48331412]What's with it? It's just smaller than any other number. Can be protected against by simple "a < 0" check that you most likely already have. Even without a special case in tonumber fixer it most likely won't cause problems.[/QUOTE]

it's not limited to people checking if it's above zero or not.. sometimes they input negative numbers.

[editline]a[/editline]

*on purpose

[editline]b[/editline]

on second thought you should really just use the IsFinite function on the return, the following work in tonumber:

] m> tonumber"nan", tonumber"inf", tonumber"-nan", tonumber"-inf", tonumber"+nan", tonumber"+inf"
nan	inf	nan	-inf	nan	inf	

[editline]c[/editline]

] m> tonumber"1e100000000"
inf	

[FPtje]

inf is not a problem. It adheres to the most important mathematical rules like the infinitely good little boy it is.

[Valkyrie]

This is what I use.

Function

function isValidNumber( number )
	number = tonumber(number)

	if ( number != number ) then	
		return false
	end

	if( !number || !isnumber(number) || number <= 0 ) then
		return false
	end

	if ( number == math.huge || number == -math.huge) then
		return false
	end

	return true	
end

Test

concommand.Add("nanTest", function()
	print( _VERSION )

	print(isValidNumber(1/0))
	print(isValidNumber(-1/0))
	print(isValidNumber((1/0)/(1/0)))
	print(isValidNumber((0/0)/(0/0)))
	print(isValidNumber(0/0))

	print(isValidNumber(10))
	print(isValidNumber(45))
	print(isValidNumber(45))
end)	

Results

] nanTest 
Lua 5.1
false
false
false
false
false
true
true
true