PHP nested menu security question

Question

From a novice:

In looking to display a modified nested menu of How to create a nested menu from MySQL with PHP?. My question is: are there any security concerns in taking this kind of approach. From my novice point of view this code is server-sided with the exception of triggering the query upon the page being loaded.

All insights and suggestions are welcome. Thank you.

<?php
include '../data.php'; // connection folder  

$query = "SELECT `parent_name`, `parent_id`, `child_name`, child_id
            FROM  `pages.child` INNER JOIN `pages.parent`
            ORDER BY `parent_name`";

$result = mysql_query($query) or die(mysql_error());
echo "<ul id=\"catmenu\">";
$last_parent = '';
while($row = mysql_fetch_array($result)){
    if($last_parent != $row['parent_name']){
            // Unless this is the first item, close the last category
            if($last_parent != ''){
                    echo "</ul></li>";
            }
            // Parent menu begins <li> and <ul>
            $last_parent = $row['parent_name'];
            $tags = $row['parent_name'];               
            echo "<a href=\"$tags\"><li class=\"menulist\">{$tags}<ul></a>";
    }
    if($row['parent_id'] === $row['child_id'] ){
        $tags = $row['parent_name'];
        $tag = $row['child_name'];
        echo "<li class=\"menulist\"><a href=\"$tags\\$tag\">$tag</a>";
        }        
}
if($last_parent != ''){
    echo "</ul></li>";
}
echo "</ul>";

?>

I would have used single quotes for echo statements. I think it's more readably without all those escape backslashes. But that is propably just a matter of taste. `echo '
You cannot wrap a `UL` in a `A`; `A` does only allow inline contents. — Gumbo, Jul 05 '11 at 05:59

EdoDodo · Accepted Answer · 2011-07-05T05:52:11.453

3

If the values in the database table were previously inputted by users, make sure you escape them using htmlentities() before outputting them. For example, replace the line:

$tags = $row['parent_name'];

With this:

$tags = htmlentities($row['parent_name']);

Using htmlentities() prevents a vulnerability known as cross-site scripting, which is the only security issue I can see in this situation.

edited Jul 05 '11 at 05:52

answered Jul 04 '11 at 18:42

EdoDodo

8,220
3
24
30

I've not used array_walk yet. Question: I'm getting an error "Warning: htmlentities() expects parameter 2 to be long, string given on line 18 – crash_course Jul 04 '11 at 19:17
How are you using htmlentities? In most cases, you'll only be passing one parameter to it, and that's the string that needs escaping. – EdoDodo Jul 04 '11 at 20:24
I'm not using it for anything other then posting it just below the while() statement. – crash_course Jul 04 '11 at 20:35
Ah, it looks like it's not possible to use `htmlentities` with `array_walk`. You'll have to use it on each element individually as you access it. I've changed my answer accordingly. – EdoDodo Jul 05 '11 at 05:52
Why `htmlentities` and not just `htmlspecialchars`? – Gumbo Jul 05 '11 at 05:58
True. In this case `htmlspecialchars` would probably be better than `htmlentities`, but I tend to use `htmlentities` since that works in all situations and with all encodings. – EdoDodo Jul 05 '11 at 06:50

PHP nested menu security question

1 Answers1