PHP Help

Hehe, I see you posted on Yahoo Answers. Let me take a better look at it and see if I can help.

 

Try that?

 

I've been out of the PHP loop for a while but, at first glance, the way the URL is built with the "$" variable within it seems dangerous.

If you are going to create a string of a URL containing a variable the way the red is built, build it before and then make it into a string. For example

Code:

< ? php $dcnCheck=$_GET['dcn' ];
$dcnCheck2="http://documents.website.com/collection/$dcnCheck.doc";
$dcnCheck3=[color=red]"http://documents.website.com/collection/$dcnCheck.docx"[/color];
if (file_exists($dcnCheck2)) {
header("Location:$dcnCheck2");
} else {
header("Location:$dcnCheck3");
}
?> 

Could be better written as

Code:

$dcnCheck2="http://documents.website.com/collection/". $dcnCheck. ".doc";

As a programmer, I always troubleshoot by writing out what the string is BEFORE I check it:

Code:

< ? php $dcnCheck=$_GET['dcn' ];
//using the concatenation method to build the DCN URL
$dcnCheck2=[color=red]"http://documents.website.com/collection/" . $dcnCheck . ".doc"[/color];
$dcnCheck3=[color=red]"http://documents.website.com/collection/" . $dcnCheck . ".docx"[/color];

[color=green]//what are we actually checking? SwoLy OutPut
echo "What we received on the GET: $dcn<br>\n";
echo "This is DCNCHECK2: $dcnCheck2<br>\n";
echo "This is DCNCHECK3: $dcnCheck3<br>\n";[/color]

if (file_exists($dcnCheck2)) {
header("Location:$dcnCheck2");
} else {
header("Location:$dcnCheck3");
}
?> 

Hope that helps a bit, Señor Pun.

EDIT: Lastly, I could have sworn that the "file_exists" function is for PATHs, not URLs. M I right?

 

Using "$dcn" and "$dcnCheck" seems confusing to me... PHP is saying, "do you want $dcn and add 'Check' at the end, or what?!? "... I think.

What I mean to say is don't add variable names inside other variable names. Calling the first variable $dcn first and then the latter $CheckedDcn or $something_new would be better.

 

I do do php for my job. You might want to put the output as a separate variable so you only have one redirect variable to worry about.

Code:

< ? php 

$dcnCheck=$_GET['dcn' ];

$dcnCheck2="/collection/" . $dcnCheck . ".doc";
$dcnCheck3="/collection/" . $dcnCheck . ".docx";

$redirectOutput= (file_exists($dcnCheck2))? $dcnCheck2$:dcnCheck3;

header("Location:".$redirectOutput);

?>

and something I notice you didn't do with your GET super-global. ALWAYS sanitize , I repeat... ALWAYS sanitize your externally defined variables!

 

hmm. that's the correct naming convention in php. maybe your IDE thinks check is a key word or something. i donno.

 

Sorry man, i pasted the wrong thing. Looks like everyone took care of it though.
I'm more of a cell phone person lol

 

note: underscores are usually used for function names. an underscore before an function name means its a standard function. these are just PHP naming conventions so they'll work regardless of how they're used. and all caps for a variable name is a global define or a super global.

 

The <i>DCN</i> number that is quite likely an input from a web form is vulnerable to a code injection attack since there is no validation -- inspection.


PHP Injection Attack

 

It means cleaning out variables that are set externally. In your case, by the uri using the $_GET super-global.

For example, if you echo the $dcnCheck variable somewhere in your code, I can inject javascript code by typing it in the uri like;

http://mysite.com?dcn=&lt;script&gt;alert('hello, as you can see cf did not sanitize this correctly, kyakko')&lt;/script&gt;

or worse, if the variable is used in a database like search string, in can inject sql code like;

http://mysite.come?dcn=1' OR 1=1

both of the code above isn't malicious but a good hacker can do bad things to your webapp.

The easiest way to sanitize a variable is strip the variable of quotes and <> and replaced them with escape characters. If you're using php5, there's a built in function called filter_var so:

$dcnCheck=filter_var($_GET['dcn']);

before php5 you'll have to either write a function or a class with a method that will do it:

Code:

function clean_var($inVar){
  $arrSearch = array("'",'"',"<",">");
  $arrReplace = array("&amp;#39;","&amp;quot;","&amp;lt;","&amp;gt;");

  return str_replace($arrSearch,$arrReplace,$inVar);
}


  $dcnCheck=clean_var($_GET['dcn']);

if you know you're incoming var is always going to be an int just use the int function:

$dcnCheck = int($_GET['dcn']);

 

Haha. That's pretty good, kyakko.

Lil Pun, write the variable's REAL string content on your code to the page before checking to make sure it's right before you try your "Location:" code.

Code:

//what are we actually checking? SwoLy OutPut
echo "What we received on the GET: \"$dcn"\n";
echo "This is DCNCHECK2: \"$dcnCheck2\"<br>\n";
echo "This is DCNCHECK3: \"$dcnCheck3\"<br>\n";

Only then you'll know what dcncheck2 and dcncheck3 contain. Troubleshoot!

 

What is the problem, though?

Try the URL the script builds without the script on a separate browser.

 

I'm guessing it's because you're using a url for the path. file_exist() is usually used for files on the server itself. Try using the local path instead of the url.

Has it worked before?

 

This messages keeps popping up every time I click on anything on this thread.

"hello, as you can see cf did not sanitize this correctly, kyakko"