From 128ac396b63159e3116e5ff9a9cf0343c15bdecc Mon Sep 17 00:00:00 2001
From: Uma <uma.s@vtiger.com>
Date: Thu, 5 Dec 2019 13:34:44 +0530
Subject: [PATCH] Fixes #1220 XSS vulnerability on ckeditor fields is addressed
---
include/utils/VtlibUtils.php | 17 +++++++++++++++--
modules/Users/actions/Save.php | 6 +++++-
modules/Vtiger/actions/Save.php | 6 ++++++
modules/Vtiger/actions/SaveAjax.php | 11 +++++++++++
packages/vtiger/optional/ModComments.zip | Bin 38785 -> 38754 bytes
.../modules/ModComments/actions/Save.php | 1 -
.../modules/ModComments/actions/SaveAjax.php | 1 -
7 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/include/utils/VtlibUtils.php b/include/utils/VtlibUtils.php
index c7f108a2b..d2ee7d15c 100644
--- a/include/utils/VtlibUtils.php
+++ b/include/utils/VtlibUtils.php
@@ -698,6 +698,10 @@ function vtlib_purify($input, $ignore=false) {
}
} else { // Simple type
$value = $__htmlpurifier_instance->purify($input);
+ global $log;
+ $log->fatal('else loop call to purifyHtmlEventAttributes');
+ $log->fatal('$value passed => ');
+ $log->fatal($value);
$value = purifyHtmlEventAttributes($value);
}
}
@@ -721,14 +725,23 @@ function purifyHtmlEventAttributes($value,$replaceAll = false){
"onselectionchange|onabort|onselectstart|onstart|onfinish|onloadstart|onshow";
// remove malicious html attributes with its value.
+ global $log;
+ $log->fatal('$replaceAll value is => ');
+ $log->fatal($replaceAll);
+ $log->fatal('$value => ');
+ $log->fatal($value);
if ($replaceAll) {
- $regex = '\s*=\s*(?:"[^"]*"[\'"]*|\'[^\']*\'[\'"]*|[^]*[\s\/>])*/i';
+ $log->fatal('if loop');
+ $regex = '\s*(=|=|&#61;|&#x26;#61;|&#61;)\s*(?:"[^"]*"[\'"]*|\'[^\']*\'[\'"]*|[^]*[\s\/>])*/i';
$value = preg_replace("/\s*(" . $htmlEventAttributes . ")" . $regex, '', $value);
} else {
- if (preg_match("/\s*(" . $htmlEventAttributes . ")\s*=/i", $value)) {
+ $log->fatal('else loop');
+ if (preg_match("/\s*(" . $htmlEventAttributes . ")\s*(=|=|&#61;|&#x26;#61;|&#61;)/i", $value)) {
$value = str_replace("=", "=", $value);
}
}
+ $log->fatal('Final value => ');
+ $log->fatal($value);
return $value;
}
diff --git a/modules/Users/actions/Save.php b/modules/Users/actions/Save.php
index 433479214..bbe106565 100644
--- a/modules/Users/actions/Save.php
+++ b/modules/Users/actions/Save.php
@@ -76,7 +76,11 @@ class Users_Save_Action extends Vtiger_Save_Action {
if ($fieldName == 'roleid' && !($currentUserModel->isAdminUser())) {
$fieldValue = null;
}
-
+ if($fieldName == 'signature' && $fieldValue !== null){
+ $purifiedContent = vtlib_purify(decode_html($fieldValue));
+ // Purify malicious html event attributes
+ $fieldValue = purifyHtmlEventAttributes(decode_html($purifiedContent),true);
+ }
if($fieldValue !== null) {
if(!is_array($fieldValue)) {
$fieldValue = trim($fieldValue);
diff --git a/modules/Vtiger/actions/Save.php b/modules/Vtiger/actions/Save.php
index 4e3ac8e5c..33a714782 100644
--- a/modules/Vtiger/actions/Save.php
+++ b/modules/Vtiger/actions/Save.php
@@ -160,6 +160,12 @@ class Vtiger_Save_Action extends Vtiger_Action_Controller {
if($fieldDataType == 'time' && $fieldValue !== null){
$fieldValue = Vtiger_Time_UIType::getTimeValueWithSeconds($fieldValue);
}
+ $ckeditorFields = array('commentcontent', 'notecontent');
+ if((in_array($fieldName, $ckeditorFields)) && $fieldValue !== null){
+ $purifiedContent = vtlib_purify(decode_html($fieldValue));
+ // Purify malicious html event attributes
+ $fieldValue = purifyHtmlEventAttributes(decode_html($purifiedContent),true);
+ }
if($fieldValue !== null) {
if(!is_array($fieldValue) && $fieldDataType != 'currency') {
$fieldValue = trim($fieldValue);
diff --git a/modules/Vtiger/actions/SaveAjax.php b/modules/Vtiger/actions/SaveAjax.php
index 49ec727da..ca95ff928 100644
--- a/modules/Vtiger/actions/SaveAjax.php
+++ b/modules/Vtiger/actions/SaveAjax.php
@@ -106,6 +106,12 @@ class Vtiger_SaveAjax_Action extends Vtiger_Save_Action {
if ($fieldDataType == 'time' && $fieldValue !== null) {
$fieldValue = Vtiger_Time_UIType::getTimeValueWithSeconds($fieldValue);
}
+ $ckeditorFields = array('commentcontent', 'notecontent', 'signature');
+ if((in_array($fieldName, $ckeditorFields)) && $fieldValue !== null){
+ $purifiedContent = vtlib_purify(decode_html($fieldValue));
+ // Purify malicious html event attributes
+ $fieldValue = purifyHtmlEventAttributes(decode_html($purifiedContent),true);
+ }
if ($fieldValue !== null) {
if (!is_array($fieldValue)) {
$fieldValue = trim($fieldValue);
@@ -138,6 +144,11 @@ class Vtiger_SaveAjax_Action extends Vtiger_Save_Action {
if ($fieldDataType == 'time' && $fieldValue !== null) {
$fieldValue = Vtiger_Time_UIType::getTimeValueWithSeconds($fieldValue);
}
+ if($fieldName == 'notecontent' && $fieldValue !== null){
+ $purifiedContent = vtlib_purify(decode_html($fieldValue));
+ // Purify malicious html event attributes
+ $fieldValue = purifyHtmlEventAttributes(decode_html($purifiedContent),true);
+ }
if ($fieldValue !== null) {
if (!is_array($fieldValue)) {
$fieldValue = trim($fieldValue);
diff --git a/packages/vtiger/optional/ModComments.zip b/packages/vtiger/optional/ModComments.zip
index 4dd6cf47a20001315c969f5234c1331e089382a0..2c2ac439ff0615df00c8ee46bdb049437ec1011c 100644
GIT binary patch
delta 2589
zcmZWr2Ut_*8qN|3OTr3<fB_{k91sO0A_&+>0U2S*2JteK5l|vm@P;WH5`KoE2wDXh
zG8AMKS+-JbMH$6G1gt1xxe6^HmgP-A4=26JbCSI8`+e_s{{PSOeJ8#S^{N)d))VD&
zCcO$dlfaLz$gzb(GUtY_C@ODFy*a)OB^)Ua4ADyeq~g$|e^UMEoiK_Fv@xb%O&b^(
zoCpL`#otQ8BKbEX4=&@?U=$gA!0&_NwUo565O7yJgoD5p%GC|U+PDpgzU`C@uqG6V
zQud_RMQFWb5Mp8oWmTCBBO#D+C=~?(f`us>cO^L44)Ns`BY{9{2N5P)K^jd;LiF~e
z*O#l|3T+Z?`fsV^y3n&UxsQ>)_Ti73mFEd7XnSi>F<v+3z<<#QgdpAC8jX=hi1FWe
znDKqSqPb=~XN6;lG){Vby`rKe5J-%m{@TZF=g{z1G?`V&&f{myZ+#1{t`z;+U!DXi
z%w>&&uT@dqG7ffmht{>Vk<Ery79M?ARI%)6)a3ZSea|fnocQBy?01`Y1ezAFER?Dh
zcMmG6$#Q#abYxETdDQy$%3;Vjr<;oxSR9dEmT~oiv5B$)-Lk6b&Ud%9yuk&C|GuT{
zm3m^S$!gG@#jRG|DSz(xWFgPKb?}6_h1m~B#bUE`pLXp{U{7EJ-I8YR&Of2)csXfK
zYmCigYUs-8p<E05dQO-6i{yOIb_lJ%-!gHHm3YABux+|Qr-fZvX-?0KndQ+m&D8BI
zN|xS@kIqVtZ0`Mq6YWDUANPc|O>K|w5|hVN*niM9nEw28AcaW7I*J#lm!$2}uaVgL
zT)M9F=F=G}<x^$jZv)4=8<L|*tAYE8uWOU1)K7=^D_-qXwJ-PNdM}m*$FcAg`!cUB
zQOM)nzIo{_mNVI9Vx^~^-#e3dE?8sr6AL&1CG3_3=h(;XF)qQBRveRhEH@SJn<S?@
zF+3;X(>h?xyNJEX%{B2!^r13+Mb7Cw(ESnL@@Tg8k~ZkJwJxYkTAYk&?i{_adDgcz
zI@a^WShC@4+r8x;We>zy%G}vwY|nJD{Bzf39Mm+BOP7jlx&G#@MbqhXQg{1ISl-_B
zCk-X#TDZJYoSwz*kSmMQq4yLcy!vis{*uS(i3_J>TKKE}_3S{MvC5!9F8WfrwZbyT
zPm|mCu!XoxeIrt@=W*JmcxYGD1N~O!Z`@5w;~5UFQL0sznNlG4{m8hx%)9S*MK!<W
zYBB7>M$7x?nn&X5crJLy+7hFVdRBg#NfSY1G_XKETC#d9sXpi=n`Mwtr|%mcxEw8|
zG_^pybFtdkZ>fGyIBI0S)Tx)vnIpPI!Hu7t&*Zp&96~ZXEmC3~OwY~+Wmz5hTu`HH
z6n30G8`G;X<#};r^|@>{IX))T{8=fpYeMU(-4b!7<indcK^>mP$(;vB$5dE)5<6`=
zlX$L&B2Okde0w1g>s5oS;q6g4@{Yna+EmENU&-D3fZp=Q0MmU|ad+28n)AZtP#p(6
zC9(Q)WnZ4;FZvTo<^kk@7nEXRSdRZ(gz~s1&!AjpehIHNARGN%tY}Eo&Zu5p$;mx#
z!{?Kdv#&P#1dSOrWmeD{{R?)m3R|YNVr(v5;0=0Ji?iGU7M=|=Gi-uS_pKxzQwcOP
z`t>>jQ)A&rH{=;JJ4W62tIUUG`iBHJR$8?`O!({Xkry|Xi9e8dmfB-@N5!A>Jvv=e
zd(7tPBNb*w#qWuu>LSR{Uc+k!H42XVT4~)@#BDqM?FWZW?w$N;WJ;#&iCu7*iWv4^
ziB8!#A32=_16H(KyWCGky-CfRBMMz=E1Eo}XRA8ydY$BIz1DuHG3H=;XLdEI>4d-G
z;p^176(2#%Va#tDAo4qgps}zCAlgn_sGmNzGlsgHti2_?#d||S58W5)g`k9?(qQF0
zx6vuTaqY)Ip(>uan;_8S$C4JRcATLPi6xBb4UfFYFsMQUHtNU*a6LP*VR-obpJ>Ry
zI6iN~;Az4b1}YVwt^_Wp%7b?)Ixvm|4y4k)W`p$@LzupvBc4Y7KQ0c90fA{78z;!E
zPqT%^k$^_JGtB!O?6<&I9T;aQz~YqvBf|oQNnkj`3YJ3x)|s|KxGU2^2=C8wg<(Mr
z%-WE<opnfP$i=6V04e(@ES3cFvK@u6bPgO7r38}>8dZRkV*;OW{GB6`1S1Rn&(cJY
zzD!00;&XhB6{ycv1^fxN<?Hkkd<#u5Ad{gAV%gGYQ#%B}F%$sXPF83`cx(c?2-2`X
zk%Ncb^xA@fJx3Mww*z2s;Achx?HmG<4#GLApo*>lOCkYEE&<i-#7|HGo1Ni)Bv8qh
z`d!u}Emz?zRX{*hd#p*WKn^mD094drzcsqpZ-agfsOX_^!J@VcEhr?9fZoLr@X8>G
zu@Nsgp;5lJHF8~JMhf4UQ$7J%1j6%oLxuS#0J`#3QNJ_6%Y5j02q+g&(T`)-O~~gr
kOgMbLH*TGOR{%XF0c0T+`5$0hs00~%6p}<-QUoXYKe0cTVgLXD
delta 2612
zcmZWr2UwHW8qQ*afPg?qXqcB(wu(wwtw=)v83Gnj1E}QDKp0-QDu$sZ7)HV&!BV9l
z5WtpYFqN&efTBFrYXOBKOQs4al!8>aNo@Xv-ak+NocDXbadOUizW0pRAku0POgp(+
z->LA><ySxjX~BxXLJ&(aW&LO?hV>Ih2=Q3p=^-_?0I5ja7W@=x1z|xNV1aTKV;%*i
zE(-y=;<U1y69K9ZinR?OER3Ajc7&YGbu1Jh@Qs0+Is{7WOC^lAb2A*76zvTZT<n#?
zY>UIi`Gxt;xa@|aZn+G?A;55>Bjj#;S_vNdOIIW~5}+*8r38Z+gE*JZK&GdPl7d@?
zI2o_Ra|TIxVeXuK)TSx0u&$u!;mkFBE9Pg~x>s?Id+Vi<`L)x_st3L0<Q)}Hkk^nf
zm@wBtZzM_+CNKCHWL)xt{rzYt!(e8jD${s%DvdcEF8-cPR)WD%GlFQ*>rU`c%)~CW
z6n=X|oh@#UPyg`1nSSzIGF&@uyBR6<W~h9FBfpXT6}#-ZPUpsXTG>3GmlAy$#dOjx
zYP_y{>|=iH(%7pvG~VWhZilWjS>2FDdfb4hmu;O??00rQzTI8x2WqVj=}9_DeqZly
zGvW@bv64^|=kzdrB3G@*-@@8{b#ddsL?1Oo@nyDa$HL9j{I>=gXvP1MYwiumC#X0Z
z$&-pGoc;1PM`GWS%NdH3t*`Wo>QN^RJxAxZ4<^qiJRbGew7;dYLwDf%ax8_G`QYoP
zT}d-l%S$hHT;!|F)nurwuef$s`r4YhjH687e{nc==PEC5Xnyb4I9r4a+#-KpR`1&8
zJHnNp&4~E>0lzq~xZK1sqoJ`sLRQd656HTfbMR=(WWJi}FH}`p#_w!Oc+|YrV8rq?
zvM1j%(7WAHWw92)zQRhhiL}>O_N<~aPz#|~7ppkogx!?|!Q<3iSM<(I7BlWx=byhf
zo?@E<Jz{f)NoF$K(+M6~<I}pxw(R9)&#LK}&o1p;EsvBtS^QI;vcrY6*UsM$)!3`Q
zX^8q3UzyXEwEl>^YOBg&ba!`t3{Q4w4|^QjQZKVe_M*}h9+uM&RxKDCT9g?dZ})#{
z9#1d}UaIjL!6{SUj!l~E|Kc^NP}6-ai>@0_)aXl!cHq-rU*eF0OtXKJy#R<9^b)iC
z<$oq;ukl%!8OKAnMst<Fe0b?xSLf(}!S2-T7lo1IKgLwyYaZMw^|g7P`>SVoUtFc3
z-7W&2pF}(DUdG@D4Huo_-orLt{LaI><P5wmCM=xLcy4Ny+7UdpQr~Cx&V^TYFEaGw
zlp=#GyLP~6H2}Ht@2=c|)cT9R-e)xSXz*3n-%s7tt01cU#i>8-`7C>-y=k~KFC{K@
zIBAAyGdB@~sSci<8M&=x+U-M#zF2!V>V}fTkn7=yK4VHnuhz=5UC*5-KajpJ9eLK;
zg6S<_R4fj3uTz_iPNrR+EugF`dilORk)Jnq+#~F?+r&yjr~e><>vw;D)Ct#V!fnl(
zriL5+Eti$*?)dk)L=WMAyZm_6BHG^3BqKg+&lTpkjSexozS}Cg?LOSijLRqM0s4o(
zc$y@I7N0gGe=nygLPvI)X4UJ`;ql0SP`fZG=X0YT_1<gL$%h8>=X<OJE5@c02X1v7
z<G#T1xx<EmgI0-~qcu6a;y06lu=M_$wk6+@^!FW$Rk@yX?$4aaGo%J{@74$D?^m1a
z53lV>bmy~z+HAO1IhH?Nv{BE_m@6Cf&$zv3$gX9-GGcHp){7i!$*oMtPG>uLhISQm
ztNTtq$!r}kAmXc^z1gd_y$4Z+F={gnMv*rb?a%*<o=~U|Ycp<LeI$T<XG5j-T$%mH
zkd1LOD)#*J@P_j9M`=BGTc4<6<F%&p7AhN3PGMg>|6`&mg1RmzY(hoV`gPVQ7);n|
zm}qd=&qCb!fUqsr`8kU*Z^_R~(&>d{bbu2?Rs<ZEkW>nmXL)E@(nL)|o}wVvjj%i!
z2ow;h+r&UnF-$z>Q7H?O__OgbP<Gy2;sFV8N}7@+Gf8=egc4xW@+ArGrjI}q0BKn|
zz&2A8^w2ED-U1sAVCbGw_>+$+V;d-?YyF@7HQn?dHjjT`O;Db>SIUM1vR4mn!Rc4O
zk;3A7Mv%^l=UJ?c0wji|lpP0}8F;B!9I(prmBMjSV;s=U4w1sbIw+A2a<bjF=-^m3
zS;`Lw&gKMe!3zTXw-O51O92uv5TENJErJ3TgA)pw^yxB#;l#QI+e~Z116Gbc5a^nc
zSknZ*(AI=Aw;_)D30!u7d8Pu$VnQ-5+Nl;M8qp}GC0C433x)#708PPh{`73R>jVIQ
zmh^7nKoJX#h!dKzfDs_Qxj3Maheq@ViKxXO?JfWRs(@My7Ey{JTcb7wR8N?QasnDs
zQ5X;d^bwIn5tTxeP<BiV7z6t7<-Gi@bMwhZBM1}#&&Plh6p6p^fVK-Lmv|8s62EnB
zJK1P>BRI&m-cq}St&e~uf%|OevIOjyIzW?Sh?u`1$|IMuHBTM~jd+tPqIx*cMM|{j
XhypbH2EZ2RKqZ_DaI$A<!gc;1zCXyd
diff --git a/pkg/vtiger/modules/ModComments/modules/ModComments/actions/Save.php b/pkg/vtiger/modules/ModComments/modules/ModComments/actions/Save.php
index 0fbd23762..49a586197 100644
--- a/pkg/vtiger/modules/ModComments/modules/ModComments/actions/Save.php
+++ b/pkg/vtiger/modules/ModComments/modules/ModComments/actions/Save.php
@@ -65,7 +65,6 @@ class ModComments_Save_Action extends Vtiger_Save_Action {
protected function getRecordModelFromRequest(Vtiger_Request $request) {
$recordModel = parent::getRecordModelFromRequest($request);
- $recordModel->set('commentcontent', $request->getRaw('commentcontent'));
$recordModel->set('reasontoedit', $request->getRaw('reasontoedit'));
return $recordModel;
diff --git a/pkg/vtiger/modules/ModComments/modules/ModComments/actions/SaveAjax.php b/pkg/vtiger/modules/ModComments/modules/ModComments/actions/SaveAjax.php
index 94f853b97..2a44e8dab 100644
--- a/pkg/vtiger/modules/ModComments/modules/ModComments/actions/SaveAjax.php
+++ b/pkg/vtiger/modules/ModComments/modules/ModComments/actions/SaveAjax.php
@@ -73,7 +73,6 @@ class ModComments_SaveAjax_Action extends Vtiger_SaveAjax_Action {
*/
public function getRecordModelFromRequest(Vtiger_Request $request) {
$recordModel = parent::getRecordModelFromRequest($request);
- $recordModel->set('commentcontent', $request->getRaw('commentcontent'));
$recordModel->set('is_private', $request->get('is_private'));
return $recordModel;
--
GitLab