It would be better if you have the CGI pass valid 'keywords', and use an internal hash that points those keywords to the right directories, such that you aren't passing a tainted variable to opendir.
Dr. Michael K. Neylon - mneylon-pm@masemware.com
||
"You've left the lens cap of your mind on again, Pinky" - The Brain
| [reply] |
I don't see why you should get flamed for asking this. On the
contrary, everyone should get feedback on security issues before
writing CGI programs :-)
My answer is: it depends on the setup, and what you do with the directory
afterwards. Judging from your previous post,
I assume that is going to be your "list of valid subdirectories".
Are these directories predefined and created by you? If
they are not (i.e. they can be created by users) then you
should be careful with them, even if they pass your "validity"
check. I think using -d is OK because it does not interpret
any metacharacters AFAIK, but if you later use that name
in something that does (like open or system), you will get
in trouble if you don't sanitize the names before allowing them
to pass.
If the valid directories can only be created by you, as long
as you are careful with their names, I think you should be
OK. In any case, it's best if you do something to untaint the
data before using it in any possibly vulnerable commands.
--ZZamboni
| [reply] |
ZZamboni: you are correct in your assumption. Yea, the directories are completely under my control, not the user's. So I need to use taint checking on the results from that directory listing too or are you saying to just untaint the vars I receive from the CGI input? I later use the directory in an open command. Is that ok? I'm a bit confused as to how far I need to go.
| [reply] |
I would sanitize everything anyway. Although the directories are
under your control, think about these:
- What if in the future you add the possibility for users
to add their own directories, and then forget to go back
and change this code to check them before using them?
- What if through some other means someone manages to add
a directory with a weird name?
I'm not sure if data read from readdir() is considered tainted
or not, but you should check everything. Come up with a regex
that describes all the valid directory names that you expect
to have, and check against that. Something like this:
foreach (@dir) {
s/^(\w+)$/$1/ || do { generate some error message, or
skip this directory };
}
Which will allow directory names with any alphanumeric characters
plus the underscore. Remember not to check for invalid
characters (you could miss some), but to only allow valid
ones (like the code above).
A similar untainting should probably be applied to user-supplied
data, plus checking that it is in the list of valid directories.
--ZZamboni
| [reply]
[d/l] |