Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

Sign In with OpenID
Advertise on LowEndTalk.com

In this Discussion

Little help with GET variables

Little help with GET variables

netomxnetomx Member
edited July 2012 in Help

I want to send a filename using GET, but I don't want ppl to try to inject code... anyone got a solution for it?

It will just have letters and space; I was thinking to use a preg_replace and then compare the original GET variable with the other one; if matches, passes the string, if not, someone tried to inject a code.

Ideas? thx!

Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
«1

Comments

  • AsadHaiderAsadHaider Member
    edited July 2012

    @netomx The following regular expression should work.. allows a-z, A-Z, 0-9, and spaces. Nothing else.

    PHP Example

    if(preg_match('/^[a-zA-Z0-9\s]+$/', $_GET['filename'])) { // valid }

  • vahevahe Member
    edited July 2012

    @AsadHaider said: if(preg_match('/^[a-zA-Z0-0\s]+$/', $_GET['filename'])) { // valid }

    Typo The second zero should be a 9: ...a-zA-Z0-9\s...

  • @vahe Haha yeah well spotted, I typed all that out pretty quickly without checking.

  • netomxnetomx Member

    and the dot?

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • netomxnetomx Member

    got it, adding a dot to the match, thanks! =D

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • NickMNickM Member

    The best solution here is probably to use a whitelist of allowed filenames that they can use, and check the input against the whitelist. Otherwise, if you're not extremely careful, you'll end up with someone using something like ../../../../../etc/passwd as the file parameter, and then you could be in for a world of hurt.

  • netomxnetomx Member

    @NickM but the preg_match above will detect the / isnt it?

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • @NickM not if it's jail'd into a directory...

    Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

  • netomxnetomx Member

    @eastonch said: @NickM not if it's jail'd into a directory...

    please explain if that applies with this:

    if(preg_match('/^[a-zA-Z0-9\s.]+$/', $_GET['video']))

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • AsadHaiderAsadHaider Member
    edited July 2012

    @netomx what exactly are you trying to do with the entire code?

  • That's a good question; at the moment, we just know that it's a form submission we're looking at.

    Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

  • netomxnetomx Member

    A video player. Will scan current directory for flv files and display them. If you click them, it will reload the page but with the flowplayer and the video you selected.

    Let me put it here:

    
    <?php
    if (isset($_GET['video']))
    if(preg_match('/^[a-zA-Z0-9\s.]+$/', $_GET['video']))
    $video=$_GET['video'];
    else
    $video="test.flv"; 
    ?>
    " style="display:block;width:624px;height:352px;position:relative; top:10%; left: 50%; margin-left:-312;" id="player">
    
    flowplayer("player", "flowplayer-3.2.12.swf");
    



    <?php exec("find . -name '*flv' -type f; find . -name '*mp4' -type f", $videos); foreach ($videos as &$lista) { $nombre=strrpos($lista, "/")+1; echo "<a style='text-decoration:none;' href='".$_SERVER['PHP_SELF']."?video=".substr($lista,2)."'>".str_replace(".", " ", substr($lista,$nombre, -4))."
    "; } ?>
    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • netomxnetomx Member
    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • Where are you building this in? -- Use NetBeans for your IDE... there's some missing Curlyz... "{""}" at lines 2,3 etc..

    So why are you worried about injection, and why are you using GET?

    Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

  • netomxnetomx Member

    @eastonch said: Where are you building this in?

    Debian 6 VPS..

    @eastonch said: So why are you worried about injection, and why are you using GET?

    It is for private use, but I don't want that a friend try to inject something. And why GET? I don't know, it's easy =P

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • eastoncheastonch Member
    edited July 2012

    Just use $_POST; they can't inject something as there's nothing in the "http://www.randomstufz.com/index.php?INJECTIONCODEBRO".

    http://www.w3schools.com/php/php_post.asp

    Submit as 'post' and retreive as $_POST['var']

    Also, pull the code off, use http://netbeans.org/downloads/ really helpful if you're getting into PHP.

    Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

  • netomxnetomx Member
    edited July 2012

    are you sure @eastonch?

    I think that someone can make a form and point the POST to my server, making that "inject proof" vulnerable

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • Only accept one host, locally?

    Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

  • netomxnetomx Member

    @eastonch said: Only accept one host, locally?

    that's one. will check that, thanks

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

  • @eastonch said: there's some missing Curlyz... "{""}" at lines 2,3 etc..

    With PHP it still works without curly brackets, depends how you format the code.

  • @AsadHaider Oh. I generally use them, mainly for syntax highlighting when editing it; makes it look a little less messy :D!

    Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

  • netomxnetomx Member

    @eastonch said: Something along these lines...

    dankeschön!

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • :']

    Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

  • netomxnetomx Member

    @eastonch remember the curlys...

    if it is just 1 line of code (example: if ($x=0) echo $var;) it works. If you need more than one line, you need to use curlies =P

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • AsadHaiderAsadHaider Member
    edited July 2012

    @netomx said: If you need more than one line, you need to use curlies =P

    Yep, so the following would work for example http://pastebin.com/sLAQqyjX

  • eastoncheastonch Member
    edited July 2012

    Oh, yeah I know that. I rarely condense my code that short. I'd rather stretch it out, for easy of reading, and i usually // comment everything too, for future reference. and then I can see for example

    <? //start vars $var1 = $_REQUEST['age']; // age var $var2 = "chris"; // Name var if ($var1 >= 17) { // test to see if age is above 17. // TRUE! +>17 } else { // FALSE! <18 } ?>

    I probs messed that up, being that i Havent been with PHP for a little while, I generally use an IDE which picks up stupid mistakes anyway.

    @asadHaider

    How do you tell it that the IF statement is finished, so your next echo "fuckpie"; isnt caught in the else for false if statment?

    Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

  • netomxnetomx Member

    @eastonch said: I probs messed that up, being that i Havent been with PHP for a little while, I generally use an IDE which picks up stupid mistakes anyway.

    I do that too, but when the code is big; if not, it is not necessary. And come on, the code is too tiny to make it bigger with comments.

    @AsadHaider said: Yep, so the following would work for example http://pastebin.com/sLAQqyjX

    that's right, but, why pie? Don't mention to @HalfEatenPie please

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • telephonetelephone Member
    edited July 2012

    Please use filter_var or filter_input. If you still wish to use regex, then use filter_var:

    filter_var($_GET['variable'], FILTER_VALIDATE_REGEXP, array('options' => array('regexp' => "/^my regex$/")));

    PHP The Right Way :)

  • AsadHaiderAsadHaider Member
    edited July 2012

    @eastonch said: How do you tell it that the IF statement is finished, so your next echo "fuckpie"; isnt caught in the else for false if statment?

    When you don't use curly brackets, only the next statement is interpreted as part of the group. If you have more than one statement, then use curly brackets.

    @telephone Hipster cat?

  • eastoncheastonch Member
    edited July 2012

    @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!

    Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

  • netomxnetomx Member

    @telephone said: Please use filter_var or filter_input. If you still wish to use regex, then use filter_var:

    what's the problem with that?

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • 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

    Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

    Thanked by 1telephone
  • @netomx said: what's the problem with that?

    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.

    @eastonch said: 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

    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.

  • vedranvedran Moderator

    How is POST safer than GET again?

  • @vedran said: 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.

  • @netomx said: A video player. Will scan current directory for flv files and display them. If you click them, it will reload the page but with the flowplayer and the video you selected.

    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.

    Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

  • debugdebug Member
    edited July 2012

    Use glob to build the filenames for the whitelist.

    < ?php
    if ( !isset($_GET['filename']) ) die('Unauthorized access.');
    
    // Build the list of files..
    $files = glob('*.flv'); // This finds all files that have a match *.flv in the current directory
    
    if ( !in_array($_GET['filename'], $files) ) die('Unauthorized access.');
    
    // send the file.
    

    Also,

    exec("find . -name '*flv' -type f; find . -name '*mp4' -type f", $videos);
    

    Really? You can use glob for that as well!

    glob('*.{flv,mp4}');
    

    Hello, World.

  • @eastonch said: That's one way; you would have to rebuild the array every time

    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....

  • yomeroyomero Member

    @debug said: Really? You can use glob for that as well!

    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.

  • netomxnetomx Member

    @debug said: Really? You can use glob for that as well!

    well I don't know glob, thank for the suggestion!

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • joepie91joepie91 Member
    edited July 2012

    @eastonch said: Just use $_POST; they can't inject something as there's nothing in the "http://www.randomstufz.com/index.php?INJECTIONCODEBRO".

    http://www.w3schools.com/php/php_post.asp

    Submit as 'post' and retreive as $_POST['var']

    Very bad advice. POST is just as injectable as GET. Nothing client-side should be trusted.

    @netomx said: http://pastebin.com/ZfYXK8SX

    There's really no reason for using exec. Use glob as @debug said.

    @netomx said: if it is just 1 line of code (example: if ($x=0) echo $var;) it works. If you need more than one line, you need to use curlies =P

    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.

    Appreciate my posts/software/guides? Donate (PayPal/Flattr/Bitcoin): http://cryto.net/~joepie91/donate.html | irc.freenode.net #lowendbox

  • netomxnetomx Member

    @joepie91 said: Use curly braces. Even if it's just one line. Not using them is a disaster waiting to happen.

    it will not happen, it is by design.

    @joepie91 said: - Please indent your code properly.

    it is not meant for public use, that's why I don't indent.

    @joepie91 said: What injection exactly are you trying to prevent? It's still not really clear to me after reading this thread.

    if you check the get, I don't know if they can put: "echo /proc/meminfo" or whatever info they need.

    Referral links: DigitalOcean referral link | Get 500MB free with Dropbox | I sell domains with Google Apps, $1 p/ user
  • @netomx said: it will not happen, it is by design.

    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.

    @netomx said: it is not meant for public use, that's why I don't indent.

    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.

    @netomx said: if you check the get, I don't know if they can put: "echo /proc/meminfo" or whatever info they need.

    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.

    Appreciate my posts/software/guides? Donate (PayPal/Flattr/Bitcoin): http://cryto.net/~joepie91/donate.html | irc.freenode.net #lowendbox

  • ReeRee Member

    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.

    Systems Administrator | IWFHosting

    Comments expressed are solely my own opinion and not of that of the companies, unless stated.

  • @eastonch said: 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

    I'm not sure how you expect to do that on a remotely accessible webpage?

    @eastonch said: for the next time he has a project there and he has a GET/POST var that could be leading to an exec / db.

    He shouldn't have any injectable parameters to begin with.

    Appreciate my posts/software/guides? Donate (PayPal/Flattr/Bitcoin): http://cryto.net/~joepie91/donate.html | irc.freenode.net #lowendbox

  • ReeRee Member
    edited July 2012

    @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.

Sign In or Register to comment.