To reiterate, I'm no expert with this stuff, but you don't have to be an expert to know how to prevent basic sql injections from happening
[editline]2nd May 2016[/editline]
[QUOTE=edgarasf123;50245052]Validate all the data, like if the price is greater than zero. Use appropriate net library functions for each data type, net.ReadFloat for numbers. And use [URL="http://wiki.garrysmod.com/page/sql/SQLStr"]sql.SQLStr[/URL] with any string variable that you're going to use in sql.Query.[/QUOTE]
I might be wrong but I was under the impression you shouldn't use sql.SQLStr while querying anything besides the built-in sqlite db
[QUOTE=timz9;50245056]To reiterate, I'm no expert with this stuff, but you don't have to be an expert to know how to prevent basic sql injections from happening
[editline]2nd May 2016[/editline]
I might be wrong but I was under the impression you shouldn't use sql.SQLStr while querying anything besides the built-in sqlite db[/QUOTE]
Oh, I thought he was using sqlite. In that case
[code]String escaped = Database:Escape( String stuff )[/code]
Doesn't this defeat the purpose of roleplay?
[QUOTE=G6Darkminion;50240520]Thanks for not being a money whore on scriptfodder.[/QUOTE]
Believe me, he's tried :v:
[QUOTE=timz9;50245017]This point has probably been beaten into you way too many times but the first thing you should do is [B]NOT TRUST THE FUCKING CLIENT[/B], and now hopefully you see why, especially with shit like this. Verify that the data you're receiving looks the way it's supposed to, escape all potentially malicious strings before inserting, don't give the client the leverage to fuck up your database. If possible, use prepared statements (even though they don't protect against EVERYTHING, they should help you avoid "stupid" injections like this), though i'm not sure again if tmysql4 can even do prepared statements.[/QUOTE]
well i have it so negative numbers can not be placed... so thats pretty much all i can do glua wise.. i dont know how to make sql safer
[QUOTE=edgarasf123;50245052]Validate all the data, like if the price is greater than zero. Use appropriate net library functions for each data type, net.ReadFloat for numbers. And use [URL="http://wiki.garrysmod.com/page/sql/SQLStr"]sql.SQLStr[/URL] with any string variable that you're going to use in sql.Query.[/QUOTE]
I do check for numbers smaller then 0 or non numbers... Now that sql.SQLstr will that return a string that is safe to use? Still not sure how that works (yes i read the wiki page)
[QUOTE=timz9;50245056]To reiterate, I'm no expert with this stuff, but you don't have to be an expert to know how to prevent basic sql injections from happening
[editline]2nd May 2016[/editline]
I might be wrong but I was under the impression you shouldn't use sql.SQLStr while querying anything besides the built-in sqlite db[/QUOTE]
This is my first addon using mysql.. i was planning on using text files but i probably would have gotten more sh*t for that... So i thank you for the help! All of you that are trying to help.. and that last part now im really confused what do i use where and why?
[QUOTE=edgarasf123;50245078]Oh, I thought he was using sqlite. In that case
[code]String escaped = Database:Escape( String stuff )[/code][/QUOTE]
So this will make it safe?
[QUOTE=Gamz365;50247106]Doesn't this defeat the purpose of roleplay?[/QUOTE]
Not really. Lets say no ones online and you want to buy something.. just check the eStore!
[QUOTE=ZeBull;50247275]Believe me, he's tried :v:[/QUOTE]
i 100% did not i dont want to make paid addons im just making addons for fun so i release everything for free now! (i have only sold 2 addons on scriptfodder) thanks tho!
[QUOTE=Handsome Matt;50251217]i mean u tried to submit a few more but they were all rejected because they were somehow worse than this script lmao[/QUOTE]
1 other script that was the jailbreaker...
[editline]3rd May 2016[/editline]
and i agree.. not my best :P
[QUOTE=XxLMM13xXx;50250584]I do check for numbers smaller then 0 or non numbers... Now that sql.SQLstr will that return a string that is safe to use? Still not sure how that works (yes i read the wiki page)
[/QUOTE]
Do not do any checks on data that is going to be sent to server, inside [URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/484f729baebf8e56c0decfaafd594b3570509dc0/lua/autorun/client/cl_init.lua#L433"]clientside code[/URL], those can be easily faked and bypassed.
And use Database:Escape( String stuff ) from tmysql page instead of sql.SQLStr. sql.SQLStr is only used with gmod sqlite which you're not using.
Why do you need to use that? Well this guy explains it [url]https://youtu.be/_jKylhJtPmI?t=143[/url] (I skipped the web history). SQL Injection is #1 reason why stuff got hacked in various places, even in some big companies like Adobe.
[QUOTE=edgarasf123;50251662]Do not do any checks on data that is going to be sent to server, inside [URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/484f729baebf8e56c0decfaafd594b3570509dc0/lua/autorun/client/cl_init.lua#L433"]clientside code[/URL], those can be easily faked and bypassed.
And use Database:Escape( String stuff ) from tmysql page instead of sql.SQLStr. sql.SQLStr is only used with gmod sqlite which you're not using.
Why do you need to use that? Well this guy explains it [url]https://youtu.be/_jKylhJtPmI?t=143[/url] (I skipped the web history). SQL Injection is #1 reason why stuff got hacked in various places, even in some big companies like Adobe.[/QUOTE]
if you look [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/484f729baebf8e56c0decfaafd594b3570509dc0/lua/autorun/server/sv_init.lua#L1055]here[/url] i have the check in serverside... the cl check is just to give a error msg if the client makes the honest mistake... And i will be adding the escape next update thanks for the info!
[QUOTE=XxLMM13xXx;50251721]if you look [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/484f729baebf8e56c0decfaafd594b3570509dc0/lua/autorun/server/sv_init.lua#L1055]here[/url] i have the check in serverside... the cl check is just to give a error msg if the client makes the honest mistake... And i will be adding the escape next update thanks for the info![/QUOTE]
[url]https://github.com/XxLMM13xXgaming/lmm_estore/blob/484f729baebf8e56c0decfaafd594b3570509dc0/lua/autorun/server/sv_init.lua#L619[/url]
[QUOTE=Handsome Matt;50256938]u do a price < 0 check clientside but not serverside
omg[/QUOTE]
Ah i see i missed a few version beta1.2 is out and solves that! Thanks!
I just recommend you go to the developer section and ask how you can improve your security before releasing the addon. The addon would be pretty nice, that is if it didn't have all these vulnerabilities. If you don't know how to keep SQL safe, ask and people will let you know. It's a harsh community, but not that harsh.
[IMG]http://i.imgur.com/QVaDc41.png[/IMG]
Just store the messages on the clientside part, and just send an int to the client to know which message to use.
[QUOTE=code_thrax;50257768]I just recommend you go to the developer section and ask how you can improve your security before releasing the addon. The addon would be pretty nice, that is if it didn't have all these vulnerabilities. If you don't know how to keep SQL safe, ask and people will let you know. It's a harsh community, but not that harsh.[/QUOTE]
Yes i missed some stuff and this is my first sql addon so i never new these problems... I am working on fixing!
[QUOTE=Lapin_b0t;50258017][IMG]http://i.imgur.com/QVaDc41.png[/IMG]
Just store the messages on the clientside part, and just send an int to the client to know which message to use.[/QUOTE]
If this just something i should just do or something that i NEED to do i dont think it affects the server
100% non-constructive criticism here. This is garbage.
[highlight](User was banned for this post ("Why reply?" - Pascall))[/highlight]
[QUOTE=Lemmie;50267515]100% non-constructive criticism here. This is garbage.[/QUOTE]
Would love to know how thanks
Hey guys! Im working on a new gui hows this look so far..?
[IMG]https://i.gyazo.com/bfe8f2964ee8d1a8975e48a17866e5b8.gif[/IMG]
I like the colors but the padding seems uneven at the bottom of the frame and the red on the hover-over button seems extra bright (compared to the blue which is pretty fantastic in my opinion).
Also, it's dashbo[b]a[/b]rd not dasbord
[QUOTE=Banana Lord.;50283884]I like the colors but the padding seems uneven at the bottom of the frame and the red on the hover-over button seems extra bright (compared to the blue which is pretty fantastic in my opinion).
Also, it's dashbo[b]a[/b]rd not dasbord[/QUOTE]
Thanks spelling error i missed haha so i should just take the red down what do you think the color should be?
I think red is fine it's just super bright is all. The blue is more toned back.
Just my opinion though.
[QUOTE=Banana Lord.;50283918]I think red is fine it's just super bright is all. The blue is more toned back.
Just my opinion though.[/QUOTE]
i think it looks pretty good i love light blue and bring red so i think it looks nice! Thanks for the feed back... im looking to win people over now! Haha
[QUOTE=XxLMM13xXx;50251721]if you look [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/484f729baebf8e56c0decfaafd594b3570509dc0/lua/autorun/server/sv_init.lua#L1055]here[/url] i have the check in serverside... the cl check is just to give a error msg if the client makes the honest mistake... And i will be adding the escape next update thanks for the info![/QUOTE]
You're not checking the lines above, the ones you insert straight into the db after reading
[code]local count = net.ReadFloat()
local class = net.ReadString()
local model = net.ReadString()
local desc = net.ReadString()
local price = net.ReadFloat()
local ent = net.ReadEntity()
local query = ("INSERT INTO shipments ( seller, count, weapon, model, description, price, pending ) VALUES ( %q, %d, %q, %q, %q, %d, 1 )"):format( ply:SteamID64(), count, class, model, desc, price )
[/code]
Just realised I've replied to an old post but [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L536]the[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L552]issue[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L568]still[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L584]exists[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L604]throughout[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L624]the[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L644]whole[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L890]codebase[/url].
[QUOTE=XxLMM13xXx;50283936]i think it looks pretty good i love light blue and bring red so i think it looks nice! Thanks for the feed back... im looking to win people over now! Haha[/QUOTE]
It's impossible to win people over if you have severe vulnerabilities in your addon. Fix all of the major vulnerabilities, then it will be worth a shot.
Like I said before, besides that, it looks amazing.
[QUOTE=Adzter;50283966]You're not checking the lines above, the ones you insert straight into the db after reading
[code]local count = net.ReadFloat()
local class = net.ReadString()
local model = net.ReadString()
local desc = net.ReadString()
local price = net.ReadFloat()
local ent = net.ReadEntity()
local query = ("INSERT INTO shipments ( seller, count, weapon, model, description, price, pending ) VALUES ( %q, %d, %q, %q, %q, %d, 1 )"):format( ply:SteamID64(), count, class, model, desc, price )
[/code]
Just realised I've replied to an old post but [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L536]the[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L552]issue[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L568]still[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L584]exists[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L604]throughout[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L624]the[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L644]whole[/url] [url=https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L890]codebase[/url].[/QUOTE]
That does not insert yet it just gets ready the check is under it.. I still have to write the string escape thing
[editline]9th May 2016[/editline]
[QUOTE=code_thrax;50284717]It's impossible to win people over if you have severe vulnerabilities in your addon. Fix all of the major vulnerabilities, then it will be worth a shot.
Like I said before, besides that, it looks amazing.[/QUOTE]
Will do thanks
[QUOTE=XxLMM13xXx;50286010]That does not insert yet it just gets ready the check is under it.. I still have to write the string escape thing
[editline]9th May 2016[/editline]
Will do thanks[/QUOTE]
I noticed it was an old post but check the links underneath what I posted, they all link to where you straight up concatenate variables into the db query, someone can just use this to completely delete your whole database.
[URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/lmm_estore_versioncheck.lua#L251-L274"]Client code on the server.[/URL]
[URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L949-L951"]You should make a function for this so you don't have to have these 3 exact lines everywhere you need to notify the client.[/URL]
[URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L5-L31"]Make a table of the network strings, loop through it instead of having this mass of calls.[/URL]
[URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L38"]A bunch of global functions starting with the same thing? Put them all in a table.[/URL]
[URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/lmm_estore_versioncheck.lua#L2-L10"]You don't need that many comments telling people not to edit that stuff.[/URL]
[URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/lmm_estore_mysql_config.lua"]This should be in a table.[/URL] I don't know why you have a ton of things that just aren't in tables. It's obvious you know what they are and how they work, but for whatever reason you just don't use them.
And jesus dude, you need to fragment your code into different files. The whole thing looks botched together.
[QUOTE=XxLMM13xXx;50286010]That does not insert yet it just gets ready the check is under it.. I still have to write the string escape thing
[editline]9th May 2016[/editline]
Will do thanks[/QUOTE]
[url]https://facepunch.com/showthread.php?t=1442438&p=46740595&viewfull=1#post46740595[/url]
[img]http://meharryp.xyz/sharex/2016/05/09/chrome_2016-05-09_14-04-54.png[/img]
it doesnt take a week to figure that out
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]
[QUOTE=meharryp;50286389][url]https://facepunch.com/showthread.php?t=1442438&p=46740595&viewfull=1#post46740595[/url]
[img]http://meharryp.xyz/sharex/2016/05/09/chrome_2016-05-09_14-04-54.png[/img]
it doesnt take a week to figure that out[/QUOTE]
Was extreamly busy sorry!
[QUOTE=man with hat;50286066][URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/lmm_estore_versioncheck.lua#L251-L274"]Client code on the server.[/URL]
[URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L949-L951"]You should make a function for this so you don't have to have these 3 exact lines everywhere you need to notify the client.[/URL]
[URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L5-L31"]Make a table of the network strings, loop through it instead of having this mass of calls.[/URL]
[URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/sv_init.lua#L38"]A bunch of global functions starting with the same thing? Put them all in a table.[/URL]
[URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/autorun/server/lmm_estore_versioncheck.lua#L2-L10"]You don't need that many comments telling people not to edit that stuff.[/URL]
[URL="https://github.com/XxLMM13xXgaming/lmm_estore/blob/master/lua/lmm_estore_mysql_config.lua"]This should be in a table.[/URL] I don't know why you have a ton of things that just aren't in tables. It's obvious you know what they are and how they work, but for whatever reason you just don't use them.
And jesus dude, you need to fragment your code into different files. The whole thing looks botched together.[/QUOTE]
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=XxLMM13xXx;50294287]VERSION 1 now out! CHeck the changelog!
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!
[/QUOTE]
You're checking if CLIENT in a file that is ran only on the server...
Making a loop for the network strings would be 10x easier, quicker, and less messy.
Creating tables and not making mass calls is good programming practice, no one likes ugly messy code.
[QUOTE=Nick78111;50294651]You're checking if CLIENT in a file that is ran only on the server...
Making a loop for the network strings would be 10x easier, quicker, and less messy.
Creating tables and not making mass calls is good programming practice, no one likes ugly messy code.[/QUOTE]
I will work on the loop and table things! I relocated the version check did not even think about that!
Sorry, you need to Log In to post a reply to this thread.