CODEWITHSUNDEEP
phpsecurityinclude
floreich
floreich

Reputation: 187

PHP Securely include files + handle invalid parameters

I'm having a little problem. I want to securely include files based on the $_GET Parameter from a subdirectory + handle if the parameter is not valid.

 <?php
if(isset($_GET['p']) && $_GET['p'] == 'fahrzeuge'){
        include 'includes/cars.php';
    }
  if(isset($_GET['p']) && $_GET['p'] == 'impressum'){
        include 'includes/impressum.php';
    }
    if(isset($_GET['p']) && $_GET['p'] == 'home'){
            include 'includes/home.php';
        }
      if(isset($_GET['p']) && $_GET['p'] == 'anfahrt'){
            include 'includes/anfahrt.php';
        }
        if(isset($_GET['p']) && $_GET['p'] == 'about'){
                include 'includes/about.php';
            }

?>

This is my Code. Sorry I know it is a noob way of solving this. How can I improve it? Any Suggestions/Help would be highly appreciated

Upvotes: 1

Views: 153

Answers (3)

Z0OM
Z0OM

Reputation: 960

This is the fastes and best way, i am a fan of short codes and found this from (HACKBUGZ PHP).

You have an array with array keys and array values.

EXAMPLE 1

<?php

    $PAGES          = array();
  
    $PAGES = [
       'home'       => 'home.html'
      ,'about'      => 'about.php'
      ,'contact'    => 'somedir/contact.php'
    ];

    @include(substr($PAGES[$_GET['p']] ?? ('home'), 0, 255));

    exit;

?>

  1. You can have different query names, filenames, filetypes and directorys.

  2. the @ catch the include error if file not exists.

  3. substr($PAGES, 0, 255) cuts the uri to 255 chars(if you dont like that just do it without)

  4. ($PAGES[$_GET['p']] ?? ('home')) checks if query (array_key) exists in array if not 'home' will be the default

EXAMPLE 2(without substr)

<?php

    $PAGES          = array();
  
    $PAGES = [
       'home'       => 'home.html'
      ,'about'      => 'about.php'
      ,'contact'    => 'somedir/contact.php'
    ];

    @include($PAGES[$_GET['p']] ?? ('home'));

    exit;

?>

Upvotes: 0

GrumpyCrouton
GrumpyCrouton

Reputation: 8621

I would use a ternary to set a variable that tells the page what to include.

This is very similar to Ofir Baruch's answer, except much shorter.

$pages = array('about','contact','home');

$p = isset($_GET['p']) && in_array($_GET['p'], $pages)? $_GET['p'] : 'home';
include "includes/{$p}.php";

Basically, you have an array of pages that are possible. In the ternary, we check if $_GET['p'] is set (isset()), AND we check if the value it contains is in the array. If it is, we use $_GET['p'] as $p, if it is not, we set $p to home, this means that home will always be the default if $_GET['p'] is not set, or not a valid page as per the array.

Upvotes: 1

Ofir Baruch
Ofir Baruch

Reputation: 10346

Set an array of legit pages. Check once if $_GET['p'] is set and if so assign its value (after escaping it) to a variable $p.

Then check if the requested page ($p) is defined in your pages array, if so - include it.

$pages = array('about','contact','home');

$p = 'home'; //Default page
if(isset($_GET['p'])) {
  $p = $_GET['p']; //no need to escape as we compare it to predefined values as @Yoshi suggested
} 

if(in_array($p, $pages)){
  include 'includes/'.$p.'.php';
} else {
   include 'includes/home.php';
}

Upvotes: 3

Related Questions