Modified how we do Select2 dynamic drop-down menus to be more secure (#9079)

* Modified how we do Select2 dynamic drop-down menus to be more secure

As noted by the author of select2, the more-secure way of creating
rich Select-dropdowns is to use jquery to create HTML snippets and
carefully modify text attributes within there. This prevents any
XSS from being brought to the page. As a side-effect, the extra
escaping that we had to do in all of the internal selectlist calls
is now no longer necessary, and has been removed. Rebased and
squashed from the original.

* Rebuilt all assets, but this still feels like it's too much stuff in here.

* Whoops, need to run that in dev, not prod
This commit is contained in:
Brady Wetherington 2021-02-02 15:55:21 -08:00 committed by GitHub
parent df4686bc96
commit 9a224a07ba
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 522 additions and 490 deletions

View file

@ -252,17 +252,17 @@ class AssetModelsController extends Controller
$assetmodel->use_text = '';
if ($settings->modellistCheckedValue('category')) {
$assetmodel->use_text .= (($assetmodel->category) ? e($assetmodel->category->name).' - ' : '');
$assetmodel->use_text .= (($assetmodel->category) ? $assetmodel->category->name.' - ' : '');
}
if ($settings->modellistCheckedValue('manufacturer')) {
$assetmodel->use_text .= (($assetmodel->manufacturer) ? e($assetmodel->manufacturer->name).' ' : '');
$assetmodel->use_text .= (($assetmodel->manufacturer) ? $assetmodel->manufacturer->name.' ' : '');
}
$assetmodel->use_text .= e($assetmodel->name);
if (($settings->modellistCheckedValue('model_number')) && ($assetmodel->model_number!='')) {
$assetmodel->use_text .= ' (#'.e($assetmodel->model_number).')';
$assetmodel->use_text .= ' (#'.$assetmodel->model_number.')';
}
$assetmodel->use_image = ($settings->modellistCheckedValue('image') && ($assetmodel->image)) ? Storage::disk('public')->url('models/'.e($assetmodel->image)) : null;

View file

@ -185,16 +185,16 @@ class UsersController extends Controller
foreach ($users as $user) {
$name_str = '';
if ($user->last_name!='') {
$name_str .= e($user->last_name).', ';
$name_str .= $user->last_name.', ';
}
$name_str .= e($user->first_name);
$name_str .= $user->first_name;
if ($user->username!='') {
$name_str .= ' ('.e($user->username).')';
$name_str .= ' ('.$user->username.')';
}
if ($user->employee_num!='') {
$name_str .= ' - #'.e($user->employee_num);
$name_str .= ' - #'.$user->employee_num;
}
$user->use_text = $name_str;

View file

@ -25,15 +25,15 @@ class SelectlistTransformer
foreach ($select_items as $select_item) {
$items_array[]= [
'id' => (int) $select_item->id,
'text' => ($select_item->use_text) ? e($select_item->use_text) : e($select_item->name),
'image' => ($select_item->use_image) ? e($select_item->use_image) : null,
'text' => ($select_item->use_text) ? $select_item->use_text : $select_item->name,
'image' => ($select_item->use_image) ? $select_item->use_image : null,
];
}
$results = [
'items' => $items_array,
'results' => $items_array,
'pagination' =>
[
'more' => ($select_items->currentPage() >= $select_items->lastPage()) ? false : true,

906
package-lock.json generated

File diff suppressed because it is too large Load diff

Binary file not shown.

BIN
public/js/dist/all.js vendored

Binary file not shown.

View file

@ -1,5 +1,5 @@
{
"/js/build/app.js": "/js/build/app.js?id=0f2b56f59544e601131b",
"/js/build/app.js": "/js/build/app.js?id=943f157c3577009c5169",
"/css/build/AdminLTE.css": "/css/build/AdminLTE.css?id=5d8ce6b758f170008cd6",
"/css/build/app.css": "/css/build/app.css?id=9b6ddbece1a3cfc99036",
"/css/build/overrides.css": "/css/build/overrides.css?id=0a65220cdae6fbb6d913",
@ -19,8 +19,8 @@
"/css/dist/skins/skin-orange-dark.css": "/css/dist/skins/skin-orange-dark.css?id=1c319a5b2494db9c85a3",
"/css/dist/skins/skin-orange.css": "/css/dist/skins/skin-orange.css?id=d1cda85cbff0723be5f7",
"/css/dist/all.css": "/css/dist/all.css?id=fc64989106daf3be016b",
"/css/blue.png": "/css/blue.png?id=4c85d6a97173123bd14a",
"/css/blue@2x.png": "/css/blue@2x.png?id=62c67c6a822439e8a4ac",
"/css/blue.png": "/css/blue.png?id=e83a6c29e04fe851f212",
"/css/blue@2x.png": "/css/blue@2x.png?id=51135dd4d24f88f5de0b",
"/css/dist/skins/skin-green-dark.min.css": "/css/dist/skins/skin-green-dark.min.css?id=26495da663d45d22a6a3",
"/css/dist/skins/skin-black-dark.min.css": "/css/dist/skins/skin-black-dark.min.css?id=789c0a0fb854092dcfb7",
"/css/dist/skins/skin-blue-dark.min.css": "/css/dist/skins/skin-blue-dark.min.css?id=f53016b009b4adab0ac0",
@ -34,5 +34,5 @@
"/css/dist/bootstrap-table.css": "/css/dist/bootstrap-table.css?id=1e77fde04b3f42432581",
"/js/build/vendor.js": "/js/build/vendor.js?id=b93877b4a88a76e1b18b",
"/js/dist/bootstrap-table.js": "/js/dist/bootstrap-table.js?id=58d95c93430f2ae33392",
"/js/dist/all.js": "/js/dist/all.js?id=7a853099c33e2d354767"
"/js/dist/all.js": "/js/dist/all.js?id=2ffa95a382ca85aa4a5f"
}

View file

@ -212,7 +212,7 @@ $(document).ready(function () {
};
return data;
},
processResults: function (data, params) {
/* processResults: function (data, params) {
params.page = params.page || 1;
@ -224,12 +224,12 @@ $(document).ready(function () {
};
return answer;
},
}, */
cache: true
},
escapeMarkup: function (markup) { return markup; }, // let our custom formatter work
templateResult: formatDatalist,
templateSelection: formatDataSelection
//escapeMarkup: function (markup) { return markup; }, // let our custom formatter work
templateResult: formatDatalistSafe,
//templateSelection: formatDataSelection
});
});
@ -331,12 +331,12 @@ $(document).ready(function () {
return loading_markup;
}
var markup = "<div class='clearfix'>" ;
markup +="<div class='pull-left' style='padding-right: 10px;'>";
var markup = '<div class="clearfix">' ;
markup += '<div class="pull-left" style="padding-right: 10px;">';
if (datalist.image) {
markup += "<div style='width: 30px;'><img src='" + datalist.image + "' style='max-height: 20px; max-width: 30px;' alt='" + datalist.text + "'></div>";
} else {
markup += "<div style='height: 20px; width: 30px;'></div>";
markup += '<div style="height: 20px; width: 30px;"></div>';
}
markup += "</div><div>" + datalist.text + "</div>";
@ -344,6 +344,58 @@ $(document).ready(function () {
return markup;
}
function formatDatalistSafe(datalist) {
// console.warn("What in the hell is going on with Select2?!?!!?!?");
// console.warn($.select2);
if (datalist.loading) {
return $('<i class="fa fa-spinner fa-spin" aria-hidden="true"></i> Loading...');
}
var root_div = $("<div class='clearfix'>") ;
var left_pull = $("<div class='pull-left' style='padding-right: 10px;'>");
if (datalist.image) {
var inner_div = $("<div style='width: 30px;'>");
/******************************************************************
*
* We are specifically chosing empty alt-text below, because this
* image conveys no additional information, relative to the text
* that will *always* be there in any select2 list that is in use
* in Snipe-IT. If that changes, we would probably want to change
* some signatures of some functions, but right now, we don't want
* screen readers to say "HP SuperJet 5000, .... picture of HP
* SuperJet 5000..." and so on, for every single row in a list of
* assets or models or whatever.
*
*******************************************************************/
var img = $("<img src='' style='max-height: 20px; max-width: 30px;' alt=''>");
// console.warn("Img is: ");
// console.dir(img);
// console.warn("Strigularly, that's: ");
// console.log(img);
img.attr("src", datalist.image );
inner_div.append(img)
} else {
var inner_div=$("<div style='height: 20px; width: 30px;'></div>");
}
left_pull.append(inner_div);
root_div.append(left_pull);
var name_div = $("<div>");
name_div.text(datalist.text);
root_div.append(name_div)
var safe_html = root_div.get(0).outerHTML;
var old_html = formatDatalist(datalist);
if(safe_html != old_html) {
console.log("HTML MISMATCH: ");
console.log("FormatDatalistSafe: ");
// console.dir(root_div.get(0));
console.log(safe_html);
console.log("FormatDataList: ");
console.log(old_html);
}
return root_div;
}
function formatDataSelection (datalist) {
// This a heinous workaround for a known bug in Select2.
// Without this, the rich selectlists are vulnerable to XSS.

View file

@ -69,7 +69,7 @@ $(function () {
};
return data;
},
processResults: function (data, params) {
/*processResults: function (data, params) {
params.page = params.page || 1;
@ -81,12 +81,12 @@ $(function () {
};
return answer;
},
},*/
cache: true
},
escapeMarkup: function (markup) { return markup; }, // let our custom formatter work
templateResult: formatDatalist,
templateSelection: formatDataSelection
//escapeMarkup: function (markup) { return markup; }, // let our custom formatter work
templateResult: formatDatalistSafe,
//templateSelection: formatDataSelection
});
});
});