0

Alright, so i am trying to do the following:

<?php include('pages/{$_GET["page"]}.txt'); ?>

Wich doesnt work. It is placed in a working site inside a , so it just leaves the div blank.

If i do this:

<?php include('pages/index.txt'); ?>

it works.

I am connecting to mydomain.com/?page=index Can anyone give me some tips here?

I tried searching, but without luck.

Guploader Upload
  • 37
  • 1
  • 6
  • you need to break out of your string first. anything between your quotes is taken literally. – ericosg Mar 03 '13 at 19:08
  • 4
    Oh, please don't do that. This is one of the most common vulnerabilities. The problem is, user can enter anything as the page parameter and thus include every accessible file from the server. – Ondřej Mirtes Mar 03 '13 at 19:08
  • 1# I just realized it was this that was wrong, i was so confused 2# Now that you say it, you are right. Is there a way to limit/fix this? I dont want to use if, as it would be a lot more manual. I want to keep it as simple as possible. – Guploader Upload Mar 03 '13 at 19:10
  • so many down-votes in both the questions and answers, people please drop a line as to why. – ericosg Mar 03 '13 at 19:10
  • the downvotes are probably because of the potential security hole for including files this way – KaeruCT Mar 03 '13 at 19:11
  • use a switch http://php.net/manual/en/control-structures.switch.php where by you can choose a page by specific arguments. – ericosg Mar 03 '13 at 19:12
  • reverse the quotes " " quotes are parsed for variables and ' ' quotes are taken litterally. if you must use ' ', take the variable out of the quotes 'pages' . $_GET['page'] . '.txt' but As Ondrej said, make sure that you do some sanitization on the file name first – Youn Elan Mar 03 '13 at 19:13
  • About the security issue, this script only loads files from the folder "pages" and only .txt files, right? Or am i missing something? – Guploader Upload Mar 03 '13 at 19:14
  • no, you can use a string like "../../etc" to include any other files (it requires a little guessing though). See my edited answer for a way to do what you want in a more secure way. – KaeruCT Mar 03 '13 at 19:15
  • that can be manipulated to read any file or folder, with adding extra / or // or ./ to simple navigate folders. One could even break out of the command and exploit even further. – ericosg Mar 03 '13 at 19:16
  • It kind of ruins the idea of it. I am trying to make it easy to add pages so you dont have to edit the source, is there a way to load valid pages from a folder? Like list contents and transform them to variables? – Guploader Upload Mar 03 '13 at 19:17
  • a way to sanitize is have an array of accepted values like this: $pages=array('page1'=>'page1.txt','page2'=>'page2.txt','index'=>'index.txt'); if(isset($_GET['page']) && isset($pages[$_GET['page']]) ) include($pages[$_GET['page']); another way is make sure it is alphanumeric with ctype_alnum – Youn Elan Mar 03 '13 at 19:17

5 Answers5

2

Edit: As many have said, doing what you want this way is unsafe. Here's an easy way to make it better:

<?php 
    $valid_pages = array('index', 'contact', 'faq');
    $page = $_GET["page"];

    if (!in_array($page, $valid_pages)) {
        $page = $valid_pages[0];
    }

    include("pages/{$page}.txt"); 
?>

This will check if the page the user wants is within the allowed pages, if not, it will use the first one in the $valid_pages array.

KaeruCT
  • 1,605
  • 14
  • 15
0

Try this:

<?php include('pages/' . $_GET["page"] . '.txt'); ?>
Gigi
  • 28,163
  • 29
  • 106
  • 188
0

First, DO NOT DO THIS! It's so insecure! I can access any file on your site if you do this. SO BAD.

Now, the actual problem with what you have is that your string is in single quotes. If you change it to double quotes, it'll work. But you still should NOT DO THIS.

$foo = "QQQ";

echo "asfd{$foo}asdf";
> asfdQQQasdf
echo 'asfd{$foo}asdf';
> asdf{$foo}asdf

One way to fix your vulnerability is to have a white list. Here's a simple example:

$file = "foo";

switch ($file) {
    case "foo":
        include ("pages/foo.txt");
        break;
    case "bar":
        include ("pages/bar.php");
        break;
    default:
        echo "YOU CANNOT ACCESS THIS FILE!";
        break;
}

Another way is to have an array of acceptable file names and check if the requested files is in that array.

sachleen
  • 30,730
  • 8
  • 78
  • 73
  • I realized, but your message seems a bit unclear to me :I. I don't really get it. – Guploader Upload Mar 03 '13 at 19:12
  • What part? I added an example of a better way to include files. – sachleen Mar 03 '13 at 19:15
  • I see now, but my idea was to make it easier to add pages, so you didnt have to edit any actualy source - is there a way to load whitelist from a folder? – Guploader Upload Mar 03 '13 at 19:18
  • Sure, that's been asked many times: http://stackoverflow.com/questions/1911382/sanitize-file-path-in-php or http://stackoverflow.com/questions/9385267/sanitizing-file-paths-in-php – sachleen Mar 03 '13 at 19:22
0

If you must do this then

<?php include('pages/' . {$_GET["page"]} . '.txt'); ?>

But as others have mentioned, this does expose a security vulnerability. At least do some checking on the page parameter to ensure it doesn't link you to files outside of the web server.

SteveP
  • 18,840
  • 9
  • 47
  • 60
0
$_GET['page'] = "index";
$allowed_page = array("index", "about", "contact");
$page = htmlentities($_GET['page']);
if((isset($_GET['page']) && $_GET['page'] != "")  && in_array($page, $allowed_page)){
$page = $_GET['page'];
}
$final_page =  'pages/' . $_GET["page"].'.txt';
include($final_page);
Steward Godwin Jornsen
  • 1,181
  • 1
  • 7
  • 13