Active directory password changer

I have written this Active Directory password changer script. Comments and testing is appreciated for things I may have overlooked, since this is my first AD Script.

There are two parts: PHP and PowerShell.

This is being run on an IIS Server with PHP 5.4.x with Fast CGI and PowerShell 2.x.

The IIS_IUSRS is a manager of a group of all people who can change their own password with this method (as I exclude any account with access to the server).

If IIS_IUSRS is not a manager of a group of people you will get "Access Denied" in the log.

My goal with this script is to change passwords of people that cannot locally access domain computers. While I want this script as secure as possible, limiting those that can change their password and not allowing those with elevated permissions to change password. I was unable to find an escape character function and had to make my own.

Feel free to test this script beyond breaking point and comment, as I would like to see how well I have done in scripting something for Active Directory.

Can anyone confirm that I don't need to escape if possible? Post results if you do break it, please.

Here is the PHP script:

```

Pw Changer

'. $_GET["successstate"] .'';
exit(header('refresh:5; ../index.php'));
}

if(!isset($_POST["submit"])){
if(!empty($_GET["errorstate"])){echo '' . $_GET["errorstate"] . '';}
// if there was no submit variable passed to the
// script (i.e. user has visited the page without clicking submit), display the form:
echo '
Username:
Old Password:
New Password:
Confirm New Password:

';

}elseif(!empty($_POST["username"]) && !empty($_POST["old_password"]) && !empty($_POST["new_password"]) && !empty($_POST["confirm"])){// Else if submit was pressed, check if all of the required variables have a value and then Use PHP to check for risks such as Username in password or useing old password
$errorstate = '';
if($_POST["new_password"] != $_POST["confirm"])

I don't really know anything about powershell, so I will only look at your PHP script.

XSS

echo ''. $_GET["successstate"] .'';

This is vulnerable to reflected XSS, with which an attacker could execute arbitrary Javascript on a victims computer (and thus steal cookies, deface the website, display a phishing form, etc). Use htmlspecialchars to prevent this (same with errorstate).

Functions

Your code isn't all that long, but for 150 lines, I would extract some code to functions to structure it. For example, displayPasswordChangeForm and processPasswordChangeForm, and checkPasswordStrength.

I would also add a logQueryResult function to avoid duplicate code:

function logQueryResult($queryString, $date, $result, $redirect, $additional) {
    $logstr = '========================================'."\r\n";
    $logstr .= '    ' . $date . ' - ' . $result . ."\r\n";
    $logstr .= '========================================'."\r\n";
    $logstr .= $_SERVER['REMOTE_ADDR'] . ' - ' . $user .": Attempted Password Change result \r\n";
    $logstr .= $query . "\r\n";
    $logstr .= $additional;
    $logstr .= "\r\n";
    file_put_contents($logfile, $logstr, FILE_APPEND | LOCK_EX);
    $errorstate = '' . $result . ': Password was changed';
    exit(header('Location: ' $redirect));
}

// use like this:
logQueryResult($query, $date, 'Success', './index.php?successstate='.$errorstate, '');
logQueryResult($query, $date, 'Failed', '.?errorstate='.$errorstate, '');

$plainUserPass = 'Username: ' .$username . "\r\n";
$plainUserPass .= 'Old Password: ' .$old_password . "\r\n";
$plainUserPass .= 'New Password: ' .$new_password . "\r\n";
logQueryResult($query, $date, 'Error Warning', '.?errorstate='.$errorstate, $plainUserPass);

Misc

• when building a string, either use all double or all single quote. Code Like $var = 'foo' . $test . "'bar'" . '\n' for example is hard to read.

• always close your curly brackets at the same time (I would close it where the opening line began, it's easier to see which block it closes that way).

• personally, I would use more spaces (before {, after } and ,, around ., etc.)

StackExchange Code Review Q#70362, answer score: 3