Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates for Operator Panel #4170

Merged
merged 2 commits into from Jun 2, 2019

Conversation

Projects
None yet
3 participants
@danbgds
Copy link
Contributor

commented May 22, 2019

No description provided.

@markjcrane markjcrane changed the title Updated fixes for CVE-2019-11409 Updates for Operator Panel May 25, 2019

@markjcrane

This comment has been minimized.

Copy link
Contributor

commented May 26, 2019

This code does not maintain the capabilities of the original code. Loose a major feature of changing the agent status. Tightens security in one area and then introduces risk in another area. As it stands I can't accept the pull request.

@danbgds

This comment has been minimized.

Copy link
Contributor Author

commented May 26, 2019

Thanks for taking a look at it. Can you clarify what you mean by "introduces risk in another area"?

All of the commented code appears to be unused/unreachable through normal use, because the front end code never uses the "action" parameter. For example, changing the agent status still works with these changes because that is handled by code in index.php rather than exec.php, with a request like /app/operator_panel/index.php?status=Do%20Not%Disturb. The code that makes the request is here:

if (sizeof($_SESSION['user']['extensions']) > 0) {
$status_options[1]['status'] = "Available";
$status_options[1]['label'] = $text['label-status_available'];
$status_options[1]['style'] = "op_btn_status_available";
if (permission_exists('operator_panel_on_demand')) {
$status_options[2]['status'] = "Available (On Demand)";
$status_options[2]['label'] = $text['label-status_on_demand'];
$status_options[2]['style'] = "op_btn_status_available_on_demand";
}
$status_options[3]['status'] = "On Break";
$status_options[3]['label'] = $text['label-status_on_break'];
$status_options[3]['style'] = "op_btn_status_on_break";
$status_options[4]['status'] = "Do Not Disturb";
$status_options[4]['label'] = $text['label-status_do_not_disturb'];
$status_options[4]['style'] = "op_btn_status_do_not_disturb";
$status_options[5]['status'] = "Logged Out";
$status_options[5]['label'] = $text['label-status_logged_out'];
$status_options[5]['style'] = "op_btn_status_logged_out";
if (is_array($status_options)) foreach ($status_options as $status_option) {
echo " <input type='button' id='".$status_option['style']."' class='btn' value=\"".$status_option['label']."\" onclick=\"send_cmd('index.php?status='+escape('".$status_option['status']."')); this.disabled='disabled'; refresh_start();\" ".$onhover_pause_refresh.">\n";
}
}
and the backend code that processes it is here:
//set user status
if (isset($_REQUEST['status']) && $_REQUEST['status'] != '') {
//update the status
$user_status = check_str($_REQUEST['status']);
$sql = "update v_users set ";
$sql .= "user_status = '".$user_status."' ";
$sql .= "where domain_uuid = '".$_SESSION['domain_uuid']."' ";
$sql .= "and user_uuid = '".$_SESSION['user']['user_uuid']."' ";
if (permission_exists("user_account_setting_edit")) {
$count = $db->exec(check_sql($sql));
}

Is that incorrect?

@markjcrane

This comment has been minimized.

Copy link
Contributor

commented May 26, 2019

You got me to look at the entire operator panel I'm rewriting a portion of it at this moment.

@markjcrane

This comment has been minimized.

Copy link
Contributor

commented May 26, 2019

I’ll review it some more. Breaking it down to components is a better way. So that each value can be validated

@markjcrane

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

Sorry my changes made the code conflict with your changes. I was just thinking of the other faults in the code that I expected you to point out later. So I started working on it.

@danbgds

This comment has been minimized.

Copy link
Contributor Author

commented May 30, 2019

If you're open to these changes, we can resubmit them against basic_operator_panel. Even if you're planning on refactoring the module in the future, it'd be beneficial to lock this stuff down in the meantime.

@markjcrane

This comment has been minimized.

Copy link
Contributor

commented May 31, 2019

@markjcrane markjcrane merged commit 932276c into fusionpbx:master Jun 2, 2019

@markjcrane

This comment has been minimized.

Copy link
Contributor

commented Jun 2, 2019

Thanks for correcting the conflict changes have been accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.