New on LowEndTalk? Please Register and read our Community Rules.
All new Registrations are manually reviewed and approved, so a short delay after registration may occur before your account becomes active.
All new Registrations are manually reviewed and approved, so a short delay after registration may occur before your account becomes active.
Comments
@Netomx
True, one liners, I generally dont. but when I'm using mysql scripts to intitially generate a DB, I like to label each one with // comment rather than rely on $querythatsetsnames instead, $query1 = "rawrshitrawr"; // this sets names in DB_Names
@AsadHaider You learn something new every day, thank you mate!
what's the problem with that?
You could use filter_var or you could be the safe way, like anybody "should" and just deny hosts except local or allowed hosts using HT.Access or plain PHP code. Then decide what's easier. Filter_Var or $_POST
They have been added by the PHP team to help you sanitize inputs. Think of it like mysql_ vs PDO. There's the old way of doing things, and then there's the new way.
It's still best to incorporate both (practice and peace of mind).
E.g. In your example, said person could be using Apache now and using .htaccess for blocking hosts, but in a month he could switch to Nginx which unless he remembered to transfer all settings over could leave his script vulnerable.
There's no need to be blocking hosts at all, this is a public page right? And GET is fine, it also means people can link to particular videos which they can't do with POST requests.
How is POST safer than GET again?
It's not safer, I guess GET is just easier to manipulate for newbies as they can just edit the url. It makes no sense to use POST with what netomx wants here.
Build an array of filenames and then check the $_GET against the array -- in_array(). If it's not in the array, unset it and carry on.
That's one way; you would have to rebuild the array every time; or you could even search all file names and insert into a DB, we're turning this into a weird project.
Use glob to build the filenames for the whitelist.
Also,
Really? You can use glob for that as well!
Yes, but it's only a dir list, which shouldn't be an expensive operation, unless the dir is very large And in addition to avoiding $_GET manipulation it also ensures that the file served actually exists....
This
What can "inject"? In a database? There is no database o_O
You mean, get a file in other directory? Then use the glob stuff.
well I don't know glob, thank for the suggestion!
Very bad advice. POST is just as injectable as GET. Nothing client-side should be trusted.
There's really no reason for using exec. Use glob as @debug said.
Use curly braces. Even if it's just one line. Not using them is a disaster waiting to happen. Just be consistent in what you do; that you are not strictly required to use something doesn't mean you shouldn't.
Anyhow, @netomx, a few things:
- Please indent your code properly. It's really not acceptable not to use tabs or spaces to indent code blocks; it makes it very hard to read your code, and as a result, very hard to spot vulnerabilities. Readability is everything.
- What injection exactly are you trying to prevent? It's still not really clear to me after reading this thread.
- Some people in this thread seem to be giving horribly bad advice. Please don't take advice here at face value, unless you understand why that advice is given and how it's better or more secure.
it will not happen, it is by design.
it is not meant for public use, that's why I don't indent.
if you check the get, I don't know if they can put: "echo /proc/meminfo" or whatever info they need.
There is no valid reason to not use curly braces 'by design'. Trying not to use braces 'because you don't need them' is a bad habit that will bite you later, when you turn one line into two lines and don't realize there are no braces. Just don't do it, don't try to find an excuse not to have to change your code if you don't have a solid reason for it.
That's not a valid reason. Part of code readability is to be able to spot bugs - especially bugs that have security-related consequences. You cannot get an overview of your code if it's one giant unindented mess, and you WILL miss important things that will fuck you over at some point.
That is only possible if you are running the input in a shell command (which you really shouldn't be doing to begin with). To sufficiently secure this particular script, just strip out all characters that are not a-z, A-Z, 0-9 or a dot. That will prevent people from using the input field to navigate to other directories. This will introduce a limitation however, in the sense that you can only access files that are in the same directory as your script - but that doesn't seem to be an issue here.
As joepie91 pointed out, POST is not any more secure than GET. An analogy would be to say that passing the filename via GET is like storing passwords in a database in clear text. The extra security added by switching to POST would be like the added security of ROT13'ing the passwords in the database. Someone intelligent enough to steal the contents of the database will surely know how to un-ROT13 the passwords. Similarly someone smart enough to do anything malicious with the filename on the querystring will surely know how to do the same with form fields.
The glob()/in_array() suggestion is the only safe solution in my opinion. preg_* may work now, and if you are the only contributor to the codebase, may work forever, but if it ever becomes a shared codebase then you know some clever fellow will eventually discover that he is unable to retrieve files with underscores in their name (or some other character you didn't think to allow), and either break your regex when he tries to allow them, or remove the preg_* call altogether because "it's broken anyway, so what could it hurt to remove it".
Personally I would (and have, since I built something similar) take it a bit further, in the interest of future proofing. As someone suggested, store the filename in a database, and along with it store a unique access string (auto incremented number, md5() of filename, completely random string of characters, just whatever suits your fancy). Pass this access string on the querystring, and then use that to lookup the filename when they request something.
Why do it that way? Because if the scope of your project expands, then you may find that you have tens of thousands of files spread out across hundreds of directories. Now all of a sudden you WILL want to be able to pass a directory separating character in the filename, since they won't all be stored in a single directory anymore. But of course you can't pass a directory separating character in a safe way (or not one that will be safe from the clever fellow I mentioned earlier), so the unique access string can be used instead of the filename.
Basically you'll end up with all the safety of the glob()/in_array() solution with the ability to scale out for a longer period of time.
If he was to use
glob()/in_array()
then I would still suggest locking his site down to only accepting GET, POST,requests
from local sources only, it's better to be safe than sorry, for the next time he has a project there and he has a GET/POST var that could be leading to an exec / db.I'm not sure how you expect to do that on a remotely accessible webpage?
He shouldn't have any injectable parameters to begin with.
@eastonch Do you mean setup the webserver to only accept traffic from 127.0.0.1? That'd work, unless as @joepie91 said, it needs to be remotely accessible (which it sounds like it needs to be).
Instead do you mean only accept requests that originated from one of @netomx's pages, and not from some other site where a malicious form has been put in place? If so, that's useless since:
1) You'd do that with HTTP_REFERER, which is provided by the client, so the attacker would just fake it to make it look like the form submission came from @netomx's page.
2) The attacker doesn't even need to setup a malicious form -- using any number of firefox plugins (firebug or tamper data come to mind) you can manipulate what your client sends to the server, so you can use the legitimate form to send illegitimate data.
No, @ree @joepie91 ;
OP Mentioned that the POST could be used maliciously via a remote-party using a phony login.php page with a form that links to his website; having something like http://stackoverflow.com/questions/9872751/accepting-get-post-requests-only-from-localhost would prevent this. -- There was a .htaccess method there too.
@eastonch netomx's problem is an apple and the link you pointed to is an orange.
Allowing only 127.0.0.1 to POST to his form doesn't just stop remote parties using a phony login.php page, it also stops everybody from using netomx's legitimate login.php page, which isn't ideal since he wants to share this with his friend.
EDIT: Added to clarify:
Let's say netomx's server is IP address 1.1.1.1, my server hosting a phony login.php is 2.2.2.2, and my personal IP address is 3.3.3.3
Whether I POST to netomx's script via his legitimate script, or via my phony script, the IP address netomx's server will see is my 3.3.3.3 address. So limiting requests to those originating from 1.1.1.1 will fail.
Won't it see the location of the actual login.php; weather it's on the same network as the POST catch?
Do you actually understand how HTTP headers are constructed, or...?
Well @joepie91 , question, can you fake the address? Look at this example
btw, I think that with a die("No allowed"); is enough
Half the stuff in this thread makes no sense whatsoever..
@netomx Did you get an something working in the end?
@netomx No, you can't fake the address, but that doesn't do what you think it does.
@AsadHaider it works with the 1st suggestion. Will improve t wuth the other suggestions on the night. Thanks!
@Ree really? will check tonight