[img]http://i.imgur.com/ouFk7tX.png[/img]
You escaped the string you received FROM the database but not the string you inserted INTO the database. :what:
[QUOTE=AK to Spray;50294785][img]http://i.imgur.com/ouFk7tX.png[/img]
You escaped the string you received FROM the database but not the string you inserted INTO the database. :what:[/QUOTE]
The issue is people being able to set their name into an injection form, therefore modifying the database. What you did was not only unneeded, but provides no security still.
[QUOTE=xbeastguyx;50294835]The issue is people being able to set their name into an injection form, therefore modifying the database. What you did was not only unneeded, but provides no security still.[/QUOTE]
The id is auto... The only info a player can insert is the description...
[QUOTE=XxLMM13xXx;50294977]The id is auto... The only info a player can insert is the description...[/QUOTE]
You dont get it. What if i send a fake message with an id i choose? You have no checks, so it'll just run.
dont trust the client. Always believe they can change anything. They arent limited to a form you provide.
[QUOTE=XxLMM13xXx;50294977]The id is auto... The only info a player can insert is the description...[/QUOTE]
[QUOTE=KingofBeast;50240590][lua]
net.Start("LMMESTOREPickupShipment")
net.WriteString("0; DROP TABLE ammo, banned, pickup, players, shipments, subscriptions, weapons")
net.SendToServer()
[/lua][/QUOTE]
[QUOTE=ms333;50295077]You dont get it. What if i send a fake message with an id i choose? You have no checks, so it'll just run.
dont trust the client. Always believe they can change anything. They arent limited to a form you provide.[/QUOTE]
the auto is always writen as null so thats never defined
[QUOTE=StonedPenguin;50295093][/QUOTE]
Will test that when i get back home but i dont know how that will work...
[QUOTE=XxLMM13xXx;50294287]VERSION 1 now out! CHeck the changelog!
[code]
V1.0
+ GUI change! Looks very good now!
+ Exploit fixes!
+ mysql string escape now added!
+ Fixed buying from server errors!
BetaV1.2
+ Added price checks
BetaV1.1
+ Fixed exploit in the NPC files
[/code]
Was extreamly busy sorry!
Not sure what you mean by the first point...
I might make a function so devs can call the eStore notify
Whats wrong with mass calls?
Why does everything need to be in a table
Why not someone will edit it and not get update notifications
again... Why does everything need to be in a table
I do thank you for looking through my code and giving feedback!
[editline]10th May 2016[/editline]
New video up! In OP and click [URL="https://youtu.be/GX6gXnd_DSU"]here[/URL][/QUOTE]
You had client code on the server. I don't know how else you want me to explain it.
The notify code should be a function so you don't have to type the same thing out every time.
It looks neater to have the strings in a table.
It doesn't need to be in a table, it doesn't make sense for it not to be. You should not be making global functions for no reason. Tables are used to organize things. It is not organized to have global functions that all serve a similar purpose not be grouped together.
You don't need a bunch of comments that say the same thing. Something as simple as this would suffice.
[lua]-- Do not edit any of these variables.
local
local
local
local
local[/lua]
"version 1 is now out"
[QUOTE]local id = net.ReadString()
LMMESTOREdb:Query("SELECT * FROM pickup WHERE id = "..id.." AND buyer = "..ply:SteamID64(), function(result)[/QUOTE]
version 1 is still shit
I don't agree with a lot of hate that this is getting. Yes, there are quite a few issues and it isn't really coded to a professional standard. I think maybe instead of a release you should have posted it as a "feedback request" rather than a "product". Other than that I think you tried well and to be honest it's not the worst thing I've seen. Definitely not the worst release either.
[QUOTE=timz9;50311016]"version 1 is now out"
version 1 is still shit[/QUOTE]
How is it still shot and how is that a exploit?!?
[editline]13th May 2016[/editline]
[QUOTE=SatoshiAaron;50311884]I don't agree with a lot of hate that this is getting. Yes, there are quite a few issues and it isn't really coded to a professional standard. I think maybe instead of a release you should have posted it as a "feedback request" rather than a "product". Other than that I think you tried well and to be honest it's not the worst thing I've seen. Definitely not the worst release either.[/QUOTE]
Thanks!
[QUOTE=XxLMM13xXx;50312040]How is it still shot and how is that a exploit?!?
[editline]13th May 2016[/editline]
Thanks![/QUOTE]
Alright dude it's obvious you don't know what the fuck you're doing with net and mysql. Go read a book about SQL injection. Google it. Do something other than act puzzled. You're putting peoples' servers in danger by posting this.
[QUOTE=man with hat;50312165]Alright dude it's obvious you don't know what the fuck you're doing with net and mysql. Go read a book about SQL injection. Google it. Do something other than act puzzled. You're putting peoples' servers in danger by posting this.[/QUOTE]
As I have stated.. This is my first sql addin!
[QUOTE=XxLMM13xXx;50312202]As I have stated.. This is my first sql addin![/QUOTE]
That doesn't make what I said any less true.
[QUOTE=man with hat;50312211]That doesn't make what I said any less true.[/QUOTE]
It also doesn't make it anymore useful to me
[QUOTE=XxLMM13xXx;50312425]It also doesn't make it anymore useful to me[/QUOTE]
[lua]
local id = net.ReadString()
LMMESTOREdb:Query("SELECT * FROM pickup WHERE id = "..id.." AND buyer = "..ply:SteamID64(), function(result)
[/lua]
Should be
[lua]
local id = LMMESTOREGetEscapedString(net.ReadString())
LMMESTOREdb:Query("SELECT * FROM pickup WHERE id = "..id.." AND buyer = "..ply:SteamID64(), function(result)
[/lua]
Take this knowledge and apply it to EVERY value received from the client. EVERY.
[QUOTE=XxLMM13xXx;50312425]It also doesn't make it anymore useful to me[/QUOTE]
maybe have a run through this? [url]https://www.hacksplaining.com/exercises/sql-injection[/url] this site is pretty cool, and maybe it'll help explain whats wrong better (i don't really know if you get what people are telling you or not, but have a run through that anyway) - specifically when it starts showing you how the SQL is affected as you type, it's a pretty good way to visualize it, it's only the real basics but hopefully it helps you start to see how it works and why it's like incredibly important
well that and not trusting the client
[QUOTE=adamdburton;50312540][lua]
local id = net.ReadString()
LMMESTOREdbd:Query("SELECT * FROM pickup WHERE id = "..id.." AND buyer = "..ply:SteamID64(), function(result)
[/lua]
Should be
[lua]
local id = LMMESTOREGetEscapedString(net.ReadString())
LMMESTOREdb:Query("SELECT * FROM pickup WHERE id = "..id.." AND buyer = "..ply:SteamID64(), function(result)
[/lua]
Take this knowledge and apply it to EVERY value received from the client. EVERY.[/QUOTE]
[QUOTE=Smt;50312661]maybe have a run through this? [url]https://www.hacksplaining.com/exercises/sql-injection[/url] this site is pretty cool, and maybe it'll help explain whats wrong better (i don't really know if you get what people are telling you or not, but have a run through that anyway) - specifically when it starts showing you how the SQL is affected as you type, it's a pretty good way to visualize it, it's only the real basics but hopefully it helps you start to see how it works and why it's like incredibly important
well that and not trusting the client[/QUOTE]
Thank you both! This will help me thanks for providing help instead of just yelling at me!
I don't think anyone "just" yelled at you. Several times folks said you had sql injection risks and pointed out examples. At that point, Googling "gmod lua avoid sql injection" would have given you a wealth of very specific info on what it is and how to avoid it. I think the fact that you need to be led by the nose to these things is what's frustrating people. You need to take the little bit of info people give initially and run with it --- not wait for that info to pile up at your feet. Otherwise, the next time something crops up, history will repeat itself. Just my $.02
[QUOTE=man with hat;50312165]Alright dude it's obvious you don't know what the fuck you're doing with net and mysql. Go read a book about SQL injection. Google it. Do something other than act puzzled. You're putting peoples' servers in danger by posting this.[/QUOTE]
See this is what I mean. Instead of being a prick just tell him where he can improve. You ain't helpful at all.
[QUOTE=Buzzkill_HABB;50318649]I don't think anyone "just" yelled at you. Several times folks said you had sql injection risks and pointed out examples. At that point, Googling "gmod lua avoid sql injection" would have given you a wealth of very specific info on what it is and how to avoid it. I think the fact that you need to be led by the nose to these things is what's frustrating people. You need to take the little bit of info people give initially and run with it --- not wait for that info to pile up at your feet. Otherwise, the next time something crops up, history will repeat itself. Just my $.02[/QUOTE]
Thank you but by yelling i mean not giving me 100% info on it.. just saying to google gives me difrent results.. im a hands on learner not just a read i like examples once i see them i can put them into play and see how they affect thats how i learned code!
[QUOTE=SatoshiAaron;50319465]See this is what I mean. Instead of being a prick just tell him where he can improve. You ain't helpful at all.[/QUOTE]
Exactly
The reason you're receiving such extreme criticism is the fact that you have released this addon with no thought on security at all, prior to releasing this. Next time ask how you can improve the security of your scripts before giving it to people. A server owner ignorant to the extreme security flaws of the addon could add this to their server and players could fuck the server all to hell.
[QUOTE=code_thrax;50380958]The reason you're receiving such extreme criticism is the fact that you have released this addon with no thought on security at all, prior to releasing this. Next time ask how you can improve the security of your scripts before giving it to people. A server owner ignorant to the extreme security flaws of the addon could add this to their server and players could fuck the server all to hell.[/QUOTE]
Why're you bringing this back up again? There's about a thousand people that have said what you're saying. He isn't going to cooperate.
[QUOTE=code_thrax;50380958]The reason you're receiving such extreme criticism is the fact that you have released this addon with no thought on security at all, prior to releasing this. Next time ask how you can improve the security of your scripts before giving it to people. A server owner ignorant to the extreme security flaws of the addon could add this to their server and players could fuck the server all to hell.[/QUOTE]
And this helps me how...? The security of this addons is pretty good right now that i'm aware of...
[QUOTE=Mikey Howell;50381682]Why're you bringing this back up again? There's about a thousand people that have said what you're saying. He isn't going to cooperate.[/QUOTE]
Again the point of replying was..?
[QUOTE=XxLMM13xXx;50382470]And this helps me how...? The security of this addons is pretty good right now that i'm aware of...
[/QUOTE]
Maybe these [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L1129]two[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L1152]lines[/url] will make you aware of the exploits still present in this addon.
Here's another hint:
[code]
net.Start("LMMESTORESellItemWeapon")
net.WriteString("dummy")
net.WriteString("d/u/m/m/y")
net.WriteString("dummy")
net.WriteFloat(10)
net.WriteEntity(Entity(0)) -- This is the crucial line.
net.SendToServer()
[/code]
Besides that, you still have multiple spots in your code where clients can inject SQL. That horse has been beaten far past death, so I won't go into it again.
[QUOTE=Mikey Howell;50381682]Why're you bringing this back up again? There's about a thousand people that have said what you're saying. He isn't going to cooperate.[/QUOTE]
You're right. However, trying to help someone is not wrong. The reason I "brought it back up again" is to try and help him. If he doesn't listen that's his fault.
New version to increase security
[QUOTE=XxLMM13xXx;50391000]New version to increase security[/QUOTE]
[CODE]
net.Start("LMMESTOREPickupAmmo")
net.WriteString("1; INSERT INTO pickup ( id, type, seller, count, weapon, model, description, price, pending, buyer ) VALUES ( 69420, 'weapon', 'server', '1', 'weaponiwant', 'weaponmodel.mdl', 'had gubi doog', '2', '0', '" .. LocalPlayer():SteamID64() .. "'); -- ")
net.SendToServer()
net.Start("LMMESTOREPickupWeapon")
net.WriteString("69420")
net.SendToServer()
[/CODE]
Your move
[QUOTE=XxLMM13xXx;50391000]New version to increase security[/QUOTE]
[img]http://i.imgur.com/dT0pTIK.png[/img]
You're trying to escape entities now? :what: To fix that exploit I told you about in my last post, you need to verify that the entity is the one you want removed when the message is received on the server. That exploit has nothing to do with SQL and everything to do with poor security in your net receiver.
[QUOTE=AK to Spray;50395836][img]http://i.imgur.com/dT0pTIK.png[/img]
You're trying to escape entities now? :what: To fix that exploit I told you about in my last post, you need to verify that the entity is the one you want removed when the message is received on the server. That exploit has nothing to do with SQL and everything to do with poor security in your net receiver.[/QUOTE]
his escape function just returns the same shit you put in if it's not a string, :snip:
[editline]26th May 2016[/editline]
Though yes, calling the function on the entity is functionally useless in this case, and so is when he called the escape function on serverside ply:SteamID64()
I seriously hope nobody ever uses this for the sake of their server
[QUOTE=timz9;50393728][CODE]
net.Start("LMMESTOREPickupAmmo")
net.WriteString("1; INSERT INTO pickup ( id, type, seller, count, weapon, model, description, price, pending, buyer ) VALUES ( 69420, 'weapon', 'server', '1', 'weaponiwant', 'weaponmodel.mdl', 'had gubi doog', '2', '0', '" .. LocalPlayer():SteamID64() .. "'); -- ")
net.SendToServer()
net.Start("LMMESTOREPickupWeapon")
net.WriteString("69420")
net.SendToServer()
[/CODE]
Your move[/QUOTE]
Version 1.2... your move
[QUOTE=AK to Spray;50395836][img]http://i.imgur.com/dT0pTIK.png[/img]
You're trying to escape entities now? :what: To fix that exploit I told you about in my last post, you need to verify that the entity is the one you want removed when the message is received on the server. That exploit has nothing to do with SQL and everything to do with poor security in your net receiver.[/QUOTE]
#1 go to that function.. i edited that..
#2 i have a ent check function if you look below that...
[QUOTE=timz9;50395880]his escape function just returns the same shit you put in if it's not a string, :snip:
[editline]26th May 2016[/editline]
Though yes, calling the function on the entity is functionally useless in this case, and so is when he called the escape function on serverside ply:SteamID64()
I seriously hope nobody ever uses this for the sake of their server[/QUOTE]
as i promise.. we will get there one day
I have not been on my PC these last few days... but im here now :)
[B][U]VERSION 1.2 IS OUT THAT FIXES THE BUGS ABOVE PLEASE HELP ME FIND MORE![/U][/B]
Sorry, you need to Log In to post a reply to this thread.