Commit 287814bc authored by Robert Lyon's avatar Robert Lyon Committed by Gerrit Code Review

Merge "Bug 1732987: Fix user input for null chars and invalid unicode"

parents 173f4c41 5287bdf9
......@@ -58,7 +58,7 @@ else if ($delete) {
foreach ($_GET as $k => $v) {
if (preg_match('/^delete\-(\d+)$/',$k,$m)) {
$ids[] = $m[1];
if (isset($_GET['unread-' . $m[1]])) {
if (param_exists('unread-' . $m[1])) {
$deleteunread++;
}
}
......
......@@ -21,14 +21,11 @@ require_once('license.php');
define('TITLE', get_string('sitelicenses', 'admin'));
define('DEFAULTPAGE', 'home');
if ($_SERVER['REQUEST_METHOD'] === 'POST') {
if (isset($_POST['license_delete'])) {
$license_delete_array_keys = array_keys($_POST['license_delete']);
$del = array_shift($license_delete_array_keys);
delete_records('artefact_license', 'name', $del);
$SESSION->add_ok_msg(get_string('licensedeleted', 'admin'));
}
if (param_exists('license_delete')) {
$license_delete_array_keys = array_keys(param_variable('license_delete'));
$del = array_shift($license_delete_array_keys);
delete_records('artefact_license', 'name', $del);
$SESSION->add_ok_msg(get_string('licensedeleted', 'admin'));
}
if (!isset($licenses)) {
......
......@@ -209,7 +209,7 @@ function adduser_validate(Pieform $form, $values) {
$form->set_error('password', get_string('passwordinvalidform', 'auth.' . $authobj->type));
}
if (isset($_POST['createmethod']) && $_POST['createmethod'] == 'leap2a') {
if (param_exists('createmethod') && param_variable('createmethod') == 'leap2a') {
$form->set_error('firstname', null);
$form->set_error('lastname', null);
$form->set_error('email', null);
......
......@@ -419,7 +419,7 @@ function auth_setup () {
// reset the password clearing the session from usr_session.
$sessionexists = ($USER->id > 0) ? get_record('usr_session', 'usr', $USER->id, 'session', $USER->get('sessionid')) : false;
$parentuser = $USER->get('parentuser');
if (($sessionlogouttime && isset($_GET['logout']))
if (($sessionlogouttime && param_exists('logout'))
|| ($sessionexists === false && $USER->get('sessionid') != '' && empty($parentuser))
|| ($sessionexists && isset($sessionexists->useragent) && $sessionexists->useragent != $_SERVER['HTTP_USER_AGENT'])) {
// Call the authinstance' logout hook
......@@ -524,7 +524,7 @@ function auth_setup () {
}
// Check if the page is public or the site is configured to be public.
if (defined('PUBLIC') && !isset($_GET['login'])) {
if (defined('PUBLIC') && !param_exists('login')) {
return;
}
......@@ -766,7 +766,7 @@ function auth_check_required_fields() {
}
// Check if the user wants to log in anyway
if ($USER->get('passwordchange') && $USER->get('parentuser') && isset($_GET['loginanyway'])) {
if ($USER->get('passwordchange') && $USER->get('parentuser') && param_exists('loginanyway')) {
$USER->loginanyway = true;
$changepassword = false;
}
......@@ -1249,7 +1249,7 @@ function auth_get_login_form() {
if (!isset($elements[$key]) && !isset($elements['login']['elements'][$key])) {
$elements[$key] = array(
'type' => 'hidden',
'value' => $value
'value' => param_variable($key)
);
}
}
......@@ -1290,7 +1290,7 @@ function auth_get_login_form_elements() {
'type' => 'text',
'class' => 'fullwidth',
'title' => get_string('username') . ':',
'defaultvalue' => (isset($_POST['login_username'])) ? $_POST['login_username'] : '',
'defaultvalue' => param_exists('login_username') ? param_variable('login_username') : '',
'rules' => array(
'required' => true
)
......
......@@ -466,7 +466,7 @@ function saml_auth_generate_login_form() {
'type' => 'text',
'title' => get_string('username') . ':',
'description' => get_string('usernamedescription'),
'defaultvalue' => (isset($_POST['login_username'])) ? $_POST['login_username'] : '',
'defaultvalue' => param_exists('login_username') ? param_variable('login_username') : '',
'rules' => array(
'required' => true
)
......
......@@ -1168,7 +1168,7 @@ EOF;
*/
public static function login_form_elements() {
$url = get_config('wwwroot') . 'auth/saml/index.php';
if (isset($_GET['login'])) {
if (param_exists('login')) {
// We're on the transient login page. Redirect back to original page once we're done.
$url .= '?wantsurl=' . urlencode(get_full_script_path());
}
......
......@@ -540,7 +540,7 @@ class AuthXmlrpc extends Auth {
// Unset locked fields
$SESSION->clear('lockedfields');
if (isset($_GET['logout'])) {
if (param_exists('logout')) {
// Explicit logout request
$this->kill_parent($remoteusername);
redirect($this->wwwroot);
......
......@@ -528,8 +528,8 @@ class PluginBlocktypeOpenbadgedisplayer extends SystemBlocktype {
$values['badgegroup'] = array_merge($values['badgegroup'], $values[$source]);
unset($values[$source]);
}
else if (isset($_POST[$source])) {
$values['badgegroup'] = array_merge($values['badgegroup'], $_POST[$source]);
else if (param_exists($source)) {
$values['badgegroup'] = array_merge($values['badgegroup'], param_variable($source));
}
}
// check that what has been entered is allowed
......
......@@ -22,8 +22,8 @@ if ($SESSION->get('pwchangerequested')) {
die_info(get_string('pwchangerequestsent'));
}
if (isset($_GET['key'])) {
$SESSION->set('forgotpasskey', $_GET['key']);
if (param_exists('key')) {
$SESSION->set('forgotpasskey', param_alphanum('key'));
redirect('/forgotpass.php');
}
if ($SESSION->get('forgotpasskey')) {
......
......@@ -65,12 +65,12 @@ switch ($action) {
break;
case DOIMPORT_ACT:
db_begin();
if (isset($_POST['import_submit'])) {
if (param_exists('import_submit')) {
save_decisions();
// Do import and print the results
do_import();
}
else if (isset($_POST['cancel_import_submit'])) {
else if (param_exists('cancel_import_submit')) {
cancel_import();
}
db_commit();
......
......@@ -104,11 +104,11 @@ $moderators = get_column_sql(
);
// updates the selected topics as subscribed/closed/sticky
if ($membership && isset($_POST['checked'])) {
$checked = array_map('intval', array_keys($_POST['checked']));
if ($membership && param_exists('checked')) {
$checked = array_map('intval', array_keys(param_variable('checked')));
// get type based on which button was pressed
if (isset($_POST['updatetopics'])) {
$type = $_POST['type'];
if (param_exists('updatetopics')) {
$type = param_variable('type');
}
// check that user is only messing with topics from this forum
$alltopics = get_column('interaction_forum_topic', 'id', 'forum', $forumid, 'deleted', 0);
......
......@@ -88,8 +88,8 @@ function pieform_element_filebrowser(Pieform $form, $element) {
if ($config['select']) {
if (function_exists($element['selectlistcallback'])) {
if ($form->is_submitted() && $form->has_errors() && isset($_POST[$prefix . '_selected']) && is_array($_POST[$prefix . '_selected'])) {
$value = array_keys($_POST[$prefix . '_selected']);
if ($form->is_submitted() && $form->has_errors() && param_exists($prefix . '_selected') && is_array(param_array($prefix . '_selected'))) {
$value = array_keys(param_array($prefix . '_selected'));
}
else if (isset($element['defaultvalue'])) {
$value = $element['defaultvalue'];
......@@ -679,10 +679,11 @@ function pieform_element_filebrowser_doupdate(Pieform $form, $element) {
if (!empty($_FILES['userfile']['name'])) {
if (!is_array($_FILES['userfile']['name'])) {
if (!empty($_POST['_userfile']) && is_array($_POST['_userfile'])) {
if (param_exists('_userfile') && is_array(param_array('_userfile'))) {
$userfile = param_array('_userfile');
// renaming file for drag and drop
$_FILES['userfile']['name'] = $_POST['_userfile']['name'];
$_FILES['userfile']['type'] = $_POST['_userfile']['type'];
$_FILES['userfile']['name'] = $userfile['name'];
$_FILES['userfile']['type'] = $userfile['type'];
}
if (strlen($_FILES['userfile']['name']) > 1024) {
http_response_code(403);
......
......@@ -266,7 +266,7 @@ class Pieform {/*{{{*/
// @todo don't rely on submit element
throw new PieformException('Forms with spam config must have a secret and submit element');
}
$this->time = isset($_POST['__timestamp']) ? $_POST['__timestamp'] : time();
$this->time = param_exists('__timestamp') ? param_integer('__timestamp') : time();
$spamelements1 = array(
'__invisiblefield' => array(
'type' => 'text',
......
<?php
/**
*
* @package mahara
* @subpackage tests
* @author Yuliya Bozhko <yuliya.bozhko@totaralearning.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL version 3 or later
* @copyright For copyright information on Mahara, please see the README file distributed with this software.
*
*/
/**
* Test functions in lib/web.php
*/
class LibwebTest extends MaharaUnitTest {
public function testFixUtf8() {
// Make sure valid data including other types is not changed.
$this->assertSame(null, fix_utf8(null));
$this->assertSame(1, fix_utf8(1));
$this->assertSame(1.1, fix_utf8(1.1));
$this->assertSame(true, fix_utf8(true));
$this->assertSame('', fix_utf8(''));
$this->assertSame('abc', fix_utf8('abc'));
$array = array('do', 're', 'mi');
$this->assertSame($array, fix_utf8($array));
$object = new stdClass();
$object->a = 'aa';
$object->b = 'bb';
$this->assertEquals($object, fix_utf8($object));
// valid utf8 string
$this->assertSame("žlutý koníček přeskočil potůček \n\t\r", fix_utf8("žlutý koníček přeskočil potůček \n\t\r\0"));
// Invalid utf8 string.
$this->assertSame('aš', fix_utf8('a' . chr(130) . 'š'), 'This fails with buggy iconv() when mbstring extenstion is not available as fallback.');
}
public function testFixUtf8Keys() {
$obj = new stdClass();
$obj->{"a\0"} = "b\0";
// Make sure the looping of weird object property names works as expected.
foreach ($obj as $k => $v) {
$this->assertTrue("a\0" === $k); // Better use === comparison here instead of the same value assert.
$this->assertTrue("b\0" === $v); // Better use === comparison here instead of the same value assert.
}
$data = array(
"x\0" => $obj,
'a' . chr(130) . 'š' => 'a' . chr(130) . 'š',
);
$result = fix_utf8($data);
$this->assertSame(array('x', 'aš'), array_keys($result));
$this->assertTrue('aš' === $result['aš']); // Better use === comparison here instead of the same value assert.
$this->assertSame(array('a' => 'b'), get_object_vars($result['x']));
$this->assertTrue('b' === $result['x']->a); // Better use === comparison here instead of the same value assert.
$this->assertTrue(array("a\0" => "b\0") === get_object_vars($obj)); // Better use === comparison here instead of the same value assert.
}
}
......@@ -1619,7 +1619,7 @@ function param_exists($name) {
function param_variable($name) {
$args = func_get_args();
list ($value) = call_user_func_array('_param_retrieve', $args);
return $value;
return fix_utf8($value);
}
/**
......@@ -1874,6 +1874,109 @@ function param_imagesize($name) {
return $value;
}
/**
* This function returns a GET or POST array parameter as array with optional
* default. If the default isn't specified and the array parameter hasn't been sent,
* a ParameterException exception is thrown. Likewise, if the array parameter does not
* contain valid alphanumext strings, a ParameterException exception is thrown
*
* Valid characters for array values are a-z and A-Z and 0-9 and _ and - and . and / and space char
*
* @param string The GET or POST array parameter you want returned
* @param mixed [optional] the default array for this parameter
*
* @return string The value of the parameter
*
*/
function param_array($name) {
if (!empty($_POST[$name])) {
return fix_utf8($_POST[$name]);
}
else {
if (func_num_args() >= 2) {
$value = func_get_arg(1);
if ($value === null) {
return array();
}
else if (is_array($value)) {
return $value;
}
else {
throw new ParameterException("The '$name' default parameter is not an array");
}
}
else {
throw new ParameterException("Missing parameter '$name' and no default supplied");
}
}
}
/**
* Makes sure the data is using valid utf8, invalid characters are discarded.
*
* Note: this function is not intended for full objects with methods and private properties.
*
* @param mixed $value
* @return mixed with proper utf-8 encoding
*/
function fix_utf8($value) {
if (is_null($value) or $value === '') {
return $value;
}
else if (is_string($value)) {
if ((string)(int)$value === $value) {
// Shortcut.
return $value;
}
// No null bytes expected in our data, so let's remove it.
$value = str_replace("\0", '', $value);
static $buggyiconv = null;
if ($buggyiconv === null) {
$buggyiconv = (!function_exists('iconv') or @iconv('UTF-8', 'UTF-8//IGNORE', '100' . chr(130) . '€') !== '100€');
}
if ($buggyiconv) {
if (function_exists('mb_convert_encoding')) {
$subst = mb_substitute_character();
mb_substitute_character('');
$result = mb_convert_encoding($value, 'utf-8', 'utf-8');
mb_substitute_character($subst);
}
else {
// Warn admins on admin/index.php page.
$result = $value;
}
}
else {
$result = @iconv('UTF-8', 'UTF-8//IGNORE', $value);
}
return $result;
}
else if (is_array($value)) {
$newvalue = array();
foreach ($value as $k => $v) {
$newvalue[fix_utf8($k)] = fix_utf8($v);
}
return $newvalue;
}
else if (is_object($value)) {
// Use clean object to ensure no funny keys are kept.
$newvalue = array();
foreach ($value as $k => $v) {
$newvalue[fix_utf8($k)] = fix_utf8($v);
}
return (object)$newvalue;
}
else {
// This is some other type, no utf-8 here.
return $value;
}
}
/**
* Works out what size a requested image should be, based on request parameters
*
......
......@@ -82,7 +82,7 @@ else if ($delete) {
if (preg_match('/^select\-([a-zA-Z_]+)\-(\d+)$/',$k,$m)) {
$list = $m[1];
$ids[$list][] = $m[2];
if (isset($_GET['unread-' . $list . '-' . $m[2]])) {
if (param_exists('unread-' . $list . '-' . $m[2])) {
$deleteunread++;
}
}
......
......@@ -52,7 +52,7 @@ if ($delete) {
if (preg_match('/^select\-([a-zA-Z_]+)\-(\d+)$/',$k,$m)) {
$list = $m[1];
$ids[$list][] = $m[2];
if (isset($_GET['unread-' . $list . '-' . $m[2]])) {
if (param_exists('unread-' . $list . '-' . $m[2])) {
$deleteunread++;
}
}
......
......@@ -7,7 +7,7 @@ echo $form_tag;
<h3 class="panel-heading"><?php echo get_string('usercreationmethod', 'admin'); ?></h3>
<div class="panel-body">
<div class="choice">
<input type="radio" name="createmethod" class="ic"<?php if (!isset($_POST['createmethod']) || $_POST['createmethod'] == 'scratch') { ?> checked="checked"<?php } ?> id="createfromscratch" value="scratch">
<input type="radio" name="createmethod" class="ic"<?php if (!param_exists('createmethod') || param_alphanum('createmethod') == 'scratch') { ?> checked="checked"<?php } ?> id="createfromscratch" value="scratch">
<label for="createfromscratch"><?php echo get_string('createnewuserfromscratch', 'admin'); ?></label>
</div>
......@@ -27,7 +27,7 @@ echo $form_tag;
</span>
<div class="choice">
<input type="radio" name="createmethod" class="ic"<?php if (isset($_POST['createmethod']) && $_POST['createmethod'] == 'leap2a') { ?> checked="checked"<?php } ?> id="uploadleap" value="leap2a"> <label for="uploadleap"><?php echo get_string('uploadleap2afile', 'admin'); ?></label> <?php echo get_help_icon('core', 'admin', 'adduser', 'leap2afile'); ?>
<input type="radio" name="createmethod" class="ic"<?php if (param_exists('createmethod') && param_alphanum('createmethod') == 'leap2a') { ?> checked="checked"<?php } ?> id="uploadleap" value="leap2a"> <label for="uploadleap"><?php echo get_string('uploadleap2afile', 'admin'); ?></label> <?php echo get_help_icon('core', 'admin', 'adduser', 'leap2afile'); ?>
</div>
<?php echo $elements['leap2afile']['html']; ?>
<?php if (isset($elements['leap2afile']['error'])) { ?>
......
......@@ -7,7 +7,7 @@ echo $form_tag;
<h3 class="panel-heading"><?php echo get_string('usercreationmethod', 'admin'); ?></h3>
<div class="panel-body">
<div class="choice">
<input type="radio" name="createmethod" class="ic"<?php if (!isset($_POST['createmethod']) || $_POST['createmethod'] == 'scratch') { ?> checked="checked"<?php } ?> id="createfromscratch" value="scratch">
<input type="radio" name="createmethod" class="ic"<?php if (!param_exists('createmethod') || param_alphanum('createmethod') == 'scratch') { ?> checked="checked"<?php } ?> id="createfromscratch" value="scratch">
<label for="createfromscratch"><?php echo get_string('createnewuserfromscratch', 'admin'); ?></label>
</div>
......@@ -27,7 +27,7 @@ echo $form_tag;
</span>
<div class="choice">
<input type="radio" name="createmethod" class="ic"<?php if (isset($_POST['createmethod']) && $_POST['createmethod'] == 'leap2a') { ?> checked="checked"<?php } ?> id="uploadleap" value="leap2a"> <label for="uploadleap"><?php echo get_string('uploadleap2afile', 'admin'); ?></label> <?php echo get_help_icon('core', 'admin', 'adduser', 'leap2afile'); ?>
<input type="radio" name="createmethod" class="ic"<?php if (param_exists('createmethod') && param_alphanum('createmethod') == 'leap2a') { ?> checked="checked"<?php } ?> id="uploadleap" value="leap2a"> <label for="uploadleap"><?php echo get_string('uploadleap2afile', 'admin'); ?></label> <?php echo get_help_icon('core', 'admin', 'adduser', 'leap2afile'); ?>
</div>
<?php echo $elements['leap2afile']['html']; ?>
<?php if (isset($elements['leap2afile']['error'])) { ?>
......
......@@ -62,7 +62,7 @@ if ($group && !group_within_edit_window($group)) {
// be processed without having to render the other blocks.
if ($blockid = param_integer('blockconfig', 0)) {
// However, if removing a newly placed block, let it fall through to process_changes
if (!isset($_POST['cancel_action_configureblockinstance_id_' . $blockid]) || !param_integer('removeoncancel', 0) || param_integer('pieform_jssubmission', 0)) {
if (!param_exists('cancel_action_configureblockinstance_id_' . $blockid) || !param_integer('removeoncancel', 0) || param_integer('pieform_jssubmission', 0)) {
require_once(get_config('docroot') . 'blocktype/lib.php');
$bi = new BlockInstance($blockid);
// Check if the block_instance belongs to this view
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment