<?php
if (preg_match('/^[a-z0-9]+$/', $_GET['p'])) {
$page = realpath('pages/'.$_GET['p'].'.php');
$tpl = realpath('templates/'.$_GET['p'].'.html');
if ($page && $tpl) {
include $page;
include $tpl;
} else {
include('error.php');
}
}
?>
How safe would you say this is?
realpath doesn't help you at in this instance, as include can resolve the page path to the same file, no matter whether it is realpath'd or the original relative. It 'seems' to be valid, but I wouldn't trust that piece of code personally. I'm sure someone figures a neat way to inject a null-character to the input, wreaking havoc to the string termination.
What you need to do instead, to be safe, is keep a whitelist (or blacklist, if it happens to suit better, but prefer whitelists) of all allowed inputs/pages.
It seems to be safe....
But not efficient. In MVC you have the controller and view dirs preset and pre known. No point in doing a stat to check if view/controller exists.
Well, actually it realpath in this case doesn't provide any security. Actually it that case it serves no purpose at all, as include internally will expand the path. Your security here actually depends on preg_match. Note however, that regex you're using won't allow you to use an any page with upper case letter, with underscore or dash.
Anyhow, I don't think that including files based on parameters passed in request is good idea. There is something wrong with your design if you need that.
realpath() will actually have some effect in this case as it'll return FALSE if the target file does not exits. But as the other posters already said - this will not add any security to the code.
It's rather a way of error-handling in case the designated files do not exist.
Better run basename() on $_GET['p']. No directory traversal attacks for sure.