Skip to content

Commit

Permalink
web: fix various XSS vulnerabilities
Browse files Browse the repository at this point in the history
Most of these involve putting user text in error messages.
Use htmlspecialchars() for this.

filenames: require POSIX portable names
  • Loading branch information
davidpanderson authored and root committed Dec 21, 2024
1 parent 1a8200e commit 6f5fc34
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 23 deletions.
28 changes: 25 additions & 3 deletions html/inc/util_basic.inc
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,34 @@ function dtime() {
return microtime(true);
}

// security vulnerabilities and user-supplied strings:
// sources:
// GET and POST arguments
// including XML documents passed as args to RPC handlers
// cookies
//
// when used as SQL query args:
// use BoincDb::escape_string() to prevent SQL injection
// when shown as HTML output
// (e.g. 'not found' error pages, user names, forum posts)
// use htmlspecialchars() to prevent XSS
// when used as file or dir name
// use is_valid_filename()

// is $x a valid file (or dir) name?
// we want to avoid
// FS traversal, e.g. "../../foo" or "/usr/lib/..."
// shell command injection, e.g. "foo; rm*"
// XSS stuff
// let's be conservative and allow only 'POSIX fully portable filenames',
// which can have only A-Z a-z 0-9 . - _
// In some cases filenames are used on volunteer hosts,
// whose OSs may have such restrictions.
//
function is_valid_filename($x) {
if (htmlspecialchars($x) != $x) return false;
if (strstr($x, '/')) return false;
return true;
if (strlen($x)>255) return false;
// \w means A-Za-z0-9_
return preg_match('/^[\w\-.]+$/', $x);
}

?>
10 changes: 5 additions & 5 deletions html/user/get_output.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function return_error($str) {
function get_output_file($instance_name, $file_num, $auth_str) {
$result = BoincResult::lookup_name(BoincDb::escape_string($instance_name));
if (!$result) {
return_error("no job instance $instance_name");
return_error("no job instance ".htmlspecialchars($instance_name));
}
$workunit = BoincWorkunit::lookup_id($result->workunitid);
if (!$workunit) {
Expand Down Expand Up @@ -124,7 +124,7 @@ function get_wu_output_file($wu_name, $file_num, $auth_str) {
$wu_name = BoincDb::escape_string($wu_name);
$wu = BoincWorkunit::lookup("name='$wu_name'");
if (!$wu) {
return_error("no workunit $wu_name");
return_error("no workunit ".htmlspecialchars($wu_name));
}
$batch = BoincBatch::lookup_id($wu->batch);
if (!$batch) {
Expand All @@ -140,15 +140,15 @@ function get_wu_output_file($wu_name, $file_num, $auth_str) {
$fanout = parse_config(get_config(), "<uldl_dir_fanout>");
$upload_dir = parse_config(get_config(), "<upload_dir>");
if (!$wu->canonical_resultid) {
return_error("no canonical result for wu $wu->name");
return_error("no canonical result for wu ".htmlspecialchars($wu->name));
}
$result = BoincResult::lookup_id($wu->canonical_resultid);
$names = get_outfile_names($result);
$path = dir_hier_path($names[$file_num], $upload_dir, $fanout);
if (file_exists($path)) {
do_download($path);
} else {
return_error("no such file: $path");
return_error("no such file: ".htmlspecialchars($path));
}
}

Expand Down Expand Up @@ -179,7 +179,7 @@ function get_wu_output_files($wu_id, $auth_str) {
$upload_dir = parse_config(get_config(), "<upload_dir>");

if (!$wu->canonical_resultid) {
return_error("no canonical result for wu $wu->name");
return_error("no canonical result for wu ".htmlspecialchars($wu->name));
}
$result = BoincResult::lookup_id($wu->canonical_resultid);
$names = get_outfile_names($result);
Expand Down
14 changes: 13 additions & 1 deletion html/user/job_file.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
// along with BOINC. If not, see <http://www.gnu.org/licenses/>.

// Web RPCs for managing input files for remote job submission
// These support systems where users - possibly lots of them -
// process jobs without logging on to the BOINC server.
//
// Issues:
//
Expand Down Expand Up @@ -111,6 +113,9 @@ function query_files($r) {
}
$i = 0;
foreach($phys_names as $fname) {
if (!is_valid_filename($fname)) {
xml_error(-1, 'bad filename');
}
$path = dir_hier_path($fname, project_dir() . "/download", $fanout);

// if the job_file record is there,
Expand Down Expand Up @@ -192,11 +197,18 @@ function upload_files($r) {

$phys_names = array();
foreach ($r->phys_name as $cs) {
$phys_names[] = (string)$cs;
$fname = (string)$cs;
if (!is_valid_filename($fname)) {
xml_error(-1, 'bad filename');
}
$phys_names[] = $fname;
}

foreach ($_FILES as $f) {
$name = $f['name'];
if (!is_valid_filename($fname)) {
xml_error(-1, 'bad FILES filename');
}
$tmp_name = $f['tmp_name'];

if ($f['error'] != UPLOAD_ERR_OK) {
Expand Down
12 changes: 3 additions & 9 deletions html/user/openid_login.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
// You should have received a copy of the GNU Lesser General Public License
// along with BOINC. If not, see <http://www.gnu.org/licenses/>.

NOT FINISHED. DON'T USE

require 'openid.php';
include_once("../inc/boinc_db.inc");
include_once("../inc/util.inc");
Expand All @@ -25,16 +27,13 @@
function show_error($str) {
page_head("Can't create account");
echo "$str<br>\n";
echo BoincDb::error();
echo "<p>Click your browser's <b>Back</b> button to try again.\n<p>\n";
page_tail();
exit();
}

try {
$openid = new LightOpenID;
echo "<pre>";
print_r($openid); exit;
if(!$openid->mode) {
if(isset($_POST['openid_identifier'])) {
$openid->identity = $_POST['openid_identifier'];
Expand All @@ -52,7 +51,6 @@ function show_error($str) {
echo 'User has canceled authentication!';
} else {
echo 'User ' . ($openid->validate() ? $openid->identity . ' has ' : 'has not ') . 'logged in.';
//print_r($openid->getAttributes());
// Create the user in the DB
$data = $openid->getAttributes();
$email_addr = $data['contact/email'];
Expand All @@ -67,7 +65,6 @@ function show_error($str) {
error_page("Account creation is disabled");
}


// see whether the new account should be pre-enrolled in a team,
// and initialized with its founder's project prefs
//
Expand All @@ -93,10 +90,7 @@ function show_error($str) {
// if (!preg_match(INVITE_CODES, $invite_code)) {
// show_error(tra("The invitation code you gave is not valid."));
// }
//}

print_r($data);
exit();
//}

$new_name = $data['namePerson/friendly'];
if (!is_valid_user_name($new_name, $reason)) {
Expand Down
2 changes: 1 addition & 1 deletion html/user/prefs_edit.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
$action = sanitize_tags(get_str("action", true));
$subset = sanitize_tags(get_str("subset"));
$venue = sanitize_tags(get_str("venue", true));
$columns = get_str("cols", true);
$columns = get_int("cols", true);
$c = $columns?"&cols=$columns":"";
check_subset($subset);
if ($action) {
Expand Down
8 changes: 4 additions & 4 deletions html/user/submit_rpc_handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function get_wu($name) {
$wu = BoincWorkunit::lookup("name='$name'");
if (!$wu) {
log_write("no job named $name was found");
xml_error(-1, "no job named $name was found");
xml_error(-1, "job not found: ".htmlspecialchars($name));
}
return $wu;
}
Expand All @@ -46,7 +46,7 @@ function get_submit_app($name) {
$app = BoincApp::lookup("name='$name'");
if (!$app) {
log_write("no app named $name was found");
xml_error(-1, "no app named $name was found");
xml_error(-1, "app not found: ".htmlspecialchars($name));
}
return $app;
}
Expand Down Expand Up @@ -105,7 +105,7 @@ function read_input_template($app, $r) {
$x = simplexml_load_file($path);
if (!$x) {
log_write("couldn't parse input template file $path");
xml_error(-1, "couldn't parse input template file $path");
xml_error(-1, "couldn't parse input template file ".htmlspecialchars($path));
}
return $x;
} else {
Expand Down Expand Up @@ -1068,7 +1068,7 @@ function ping($r) {
$r = simplexml_load_string($req);
if (!$r) {
log_write("----- RPC request: can't parse request message: $req");
xml_error(-1, "can't parse request message: $req");
xml_error(-1, "can't parse request message: ".htmlspecialchars($req));
}

log_write("----- Handling RPC; command ".$r->getName());
Expand Down

0 comments on commit 6f5fc34

Please sign in to comment.