From ae4a0570ad47aa7e329b2b7cfb1108f92b358232 Mon Sep 17 00:00:00 2001 From: MiroslavDionisiev Date: Tue, 29 Apr 2025 09:51:04 +0300 Subject: [PATCH 1/4] feat: [UEPR-230] add sanitization when loading a project --- packages/scratch-vm/src/serialization/deserialize-assets.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/scratch-vm/src/serialization/deserialize-assets.js b/packages/scratch-vm/src/serialization/deserialize-assets.js index 568614c3dc..c1e85c6d4a 100644 --- a/packages/scratch-vm/src/serialization/deserialize-assets.js +++ b/packages/scratch-vm/src/serialization/deserialize-assets.js @@ -1,5 +1,6 @@ const JSZip = require('jszip'); const log = require('../util/log'); +const {sanitizeSvg} = require('@scratch/scratch-svg-renderer'); /** * Deserializes sound from file into storage cache so that it can @@ -156,6 +157,11 @@ const deserializeCostume = function (costume, runtime, zip, assetFileName, textL return Promise.all([textLayerFilePromise, costumeFile.async('uint8array') + .then(data => + (costumeFormat === 'svg' ? + sanitizeSvg.sanitizeByteStream(data) : + data) + ) .then(data => storage.createAsset( assetType, // TODO eventually we want to map non-png's to their actual file types? From 3ec5a14dc20079c4f9d07bcb866a7b0cdcecce50 Mon Sep 17 00:00:00 2001 From: MiroslavDionisiev Date: Wed, 7 May 2025 16:52:58 +0300 Subject: [PATCH 2/4] fix: [UEPR-230] change fixture to take into account added sanitization --- .../scratch-vm/test/fixtures/corrupt_svg.sb2 | Bin 3854 -> 2913 bytes .../scratch-vm/test/fixtures/corrupt_svg.sb3 | Bin 4856 -> 4217 bytes .../test/fixtures/corrupt_svg.sprite2 | Bin 4352 -> 3391 bytes .../test/fixtures/corrupt_svg.sprite3 | Bin 2713 -> 2138 bytes .../test/integration/offline-custom-assets.js | 3 ++- .../test/integration/sb2_corrupted_svg.js | 5 +++-- .../test/integration/sb3_corrupted_svg.js | 5 +++-- .../test/integration/sprite2_corrupted_svg.js | 5 +++-- .../test/integration/sprite3_corrupted_svg.js | 5 +++-- 9 files changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/scratch-vm/test/fixtures/corrupt_svg.sb2 b/packages/scratch-vm/test/fixtures/corrupt_svg.sb2 index 4290c1bfcda03b2e1d1b598c288e18b9494cdfe8..065f7127a7dbb1e4a6ec3086d255c8cb792a4b6b 100644 GIT binary patch delta 558 zcmeB^dnm>p;LXe;!oV>3FOT?SC0^CZ+ML1@!&b3E1Os@*H|sFQFx7*(3>*xflNUuX z_V-=A$-uy1!pOkD${@pFs8?K;9vZ^Q!2D~1NCpU(R&X;gvb-FCpIkM!X`=>qm(Hqx4XAt*VJ%90$ zyC(!SUfukWQLid@KR$nc!FslhaXKbO>x5#@o#swnI%VUX{^Ct<`ijp`F3${%x&FFeXP&$8xA$&1I& z)pq~nx_>1__T3^MQ|GtZ0(B~O+nDD>pfmTIn}ht*X7G-RN*#o2}}wqGwG; zs`V+gMK|gM{(74JVS%rPeg88`poaAHIe zf9y0>)sJa&0#_RoJ0p__Gb9#*7#Su9@G3I@1ICFqqgqISHzSiANDgc0O8~_g7=RJM zu%)pK7~ashV1>j5T0DS^f>`qnXh9zf!(;^x!^!jbIDl4$@F_ASvraDO(+6^v13BB- RCZFQ7;`#$L+n*C;2>>%Tws`;m delta 1512 zcmaDT)+Z+%;LXe;!oUH9wMEGx3?ML3_^mcO$A*hSk9Pxk?^zfauqXn`7%(v~urer2 zKF_JP*^$wlsh*u<{>RpJra)nk6hc$?l&uhxfttQ>fi*E0>J^uzhlcPnu%~5qCba`G zh%T+*W?*Fb%E-U~bQA-_(lGD*$7TZi-mg!%>dk${#Ii>Jk*!H?1Jm1;+wa|pkmk;g zZd-EX<^K0)CQNBDt9RJ$k#NL0OYVNn=LzqRO;`VTi7)wjd|B_bU4q-+8OR1+yU>w* z+g5gYtZ-o6%Uiem|Gr*#Gow+yM(@S1uQj`Rc0aT5z21NO?c1`+=E}bxW^#M2)ZL}8 zB_Hp9_vz{AE~}+mo-RMToZsnA?dF>u^W)e5HQ37j@p=5C@5dAEZRL)-)r;IVj6YFq ze)8LgJ)*ODrR8of6uql)?BAU(?|RATKOHZ3%s*1`X1+iB#~b{IKbIb|4tag<_ws4H z>wmx0*mZc_&!_6DiZ>QazI5@}62=^5yYl6i*ZmCB{QTj=g^oh0U;1y?9puPWzVOO? zVY0w2t&gi4woGI2ShT#`#s9xR?~Jy;sr7&E@IQ|&zUS}!{ny-ojvD_KY;X+ym4Eg7 z*>Cq1l$pMqt894S#8t)i^FV!m6HA9vwe~w!3Dsur8m45G4R4mP^`@&&UA}qx$&72? zjM#Q{8bw@kuy^#H@$}7PmGgD!-4kwne`B2WZ(sZ(w}i8jfm-lo=%aICubL z^{g((wtwPmG@h~e5SQQ?i$p{ql%_3L;7WV;`R>sxr*0(7Iqc;2xW_0y)HlasImd4O z!-6^=IOBDVe$82W)NFa+qG!j_rrlV!rq_Sc^Vu^_G~JARGQ-`jzbmY4qr#6wmWrsV8Q{=@%bXKp~ni8O! zRd)5c)_U68@lutH~ z4o#}R_lTc+>hrC?Sf0MR@^DwMSoXqCXM|Txcp<}KT)%kQQPDLu`%E3xo{MweVcl{1 znqA?k0)c5OET+rxEavKbz9nfXXWzul9Gd$d@_tSA5{fUcxMrjjU2|?t;+Kp~ZVK}| zdXg2o`#$_@e*XV1bNQFzFPGc*GX{9GbLiIE|SBTah?<0;d$v zI8QD`CT5PwIb2pi&T=kA<`*0clN(soCeP#I0E+Bm6X!t~<-;{umfMPp1?aRdU||5; CxpcPx diff --git a/packages/scratch-vm/test/fixtures/corrupt_svg.sb3 b/packages/scratch-vm/test/fixtures/corrupt_svg.sb3 index 52fb93096f082385fc0aec5c32df6fff96e588b7..5a7914ada10521e0081598f480b5f7911d4d35d1 100644 GIT binary patch delta 683 zcmeyN`cq*-JTC(S2Loq*e#n)yu;iGDIqL+OSwt9Mf>{h5xtlgyGcq%x2y=HFurQq* z&oogIrns^>Bjmt|U1j^37#NJ%7#P$i^K;5=HfMgzB!;YH{>RpJra&bIOp|#y6(_$D zaM`?`&728tILMfea{`Q$C$n?@mxk#)p1dgP{EyfFZv)+B&B(xD&mc3|kjtr_lYx2Z z1d$97F0J5ZU}Sm8%)kI9eEWU!h``ckMr*0l@ee~*?LZNv93s=@PJ&Dfc+o#`Fx9Pbap0rnDMMi|` zttr#ej}@JB-Y5SoIidATiq>;;zCOHqe3G%**M(6G9L8N9T7MV+TX3z;^1Z6t zB@3==7MDX$PtnQfTyM>1+PC5PhkKVc_R7bWe}1`t9GS#i#J%8vl={ij_PUHB*P_HtA`V}Lg_Ttpbog|>vm zYA(r$0S3$lAogNVn0$|4eeygZmx+6%CJXYpOity?XJnXsm9Gd$dhjbUO8{v>PMOIa z{Dwf*E`CKuoym9jqkwF40Y&C>Oc2>}0TZCe8lcE0R*1+O0Uw}$}d3j@O&ZjdnmOG4lM delta 1326 zcmeyV@I!S%JTC_W2Loq*eh32yOw3uQ$j)&kEi5?($h!l?0p83kB0wcbiZ*LAGBW~| zWifQ*ZUQP{k-(~KvOUv8O?D3Mjsq5^KvfkehE+CaKx_nx8nZDls88O{AvcLnXR|T$ zK_)eJjsqulmF;JO$RV77tjmCD@*WPw&Dw0$OhC8H|Jb_D6sX<+h_R@jyq)8}Hakbh zIRVDWKpRh?xaC}E3*0S?K)2X4C``V{=~U0lz?{+4nFpIK%n1vc=u>EtbqeUml0Y@=;M_T(F@lhe(9W-j0R+ipW=^vcOn zu1x33&prImxM%0yH_;_Vhi`m|?Xygkn7QuF>Sec|{&?Q6@AB~W?Ctqxk2AkdIc8YT zlxY5J$F^@9;?|yfZfvW*MZRqH48v_q|9m3jZ!Ozh-+5x|HSPGLFYbhFJ6Cu{#BFP4 z+1A$FmSZoj%=rBL-=t)*jU1;YIx7k~&A!FMW~wapd#_{c{a&_zC$6=!?%wA5`_Vn! z#=vijZL21^E?<-{L>d|GrPow>N$KebGyuE$^~FCbV(J+@v7lubqV?9&WArP(fL`{9j-5NgmXS`#k7FHO&y1oeOj+{WN0NX6uD!iWOyMV z=umFJ)TK6kOdcXvm3%8Am^ycQ&yH>4eDK1nBI(h_1jF~6RPO((zu$dwazxDJ?D;W) zQ#}+X-i}%3<0kWLqqIVqm+->i`B82vi(}L>t#lWhyJM(UD!|d!e&nX2_Me)E>}xKBEyRP4+-W0F(${3}nd%qfgly(B-0rBuLL z=Im3~Ij)YcYU8IDSAJW0+sjG4VvgdAxMK%S+a`4!YJ9p}RnX*e7T4=QhKg02RCH{+ zcjinEHu%K)rDd~3rNNDu?Z5K>oy{_@HT%1xFg`=#KVyJ5JIADHucn*>W+Fy5QRJKi z&m)tixi9mwbHr*c$p94*8yFZSzveDtx5= zwZOayv=sujG=AX#=}3U92DuX90dP)+DQ4KxD8j`6%GW%|`Fe7ofXHM$J~p5e__@U= bzvky+&gTWWRDH6%Kmb=g3j;$kAIJ;#cmsOUtoKsDs$x~hf*v3Mypk|ml_?eYqM1yR`jgNNVPttw&+Hkz+X?(KTHAM zu#jot0EXA$r2#!#Hy>f;VPpl1PnP8upX|aYGdYf5d9yXUITJh}n~Rb|RJ<9~dVv9X z3y8HC6eh3Z_nx>*dNMOti6qQakg`Ch{-r^{uuf)TU{Iax$SAvcFIPRIAhHs$Nt0uF zUQG_?&0}Pke2h0&Lyj3<}>XV_*Q9$grhRjB#=!FVKCA?DDLT_(O}k0B=Sn5s*=n zm+~nxdoVFfKF8`kS)Y#sXpJntB2y|e*nk**U7$c0P~bc3d%5ug$dh8CD2pePIX z$pMV4^}G!1mJ3~yY=9UCo&VZJpze9N&NEh(O*iN6 zl{d8sSkc1bzvrWv)cj*vLgCwPx743M^JZPHqbgsPsOjd{J8M3lJ-So+{=6g6);+QH zH%+5nxy9^0T;dYxtlXFTy<|?AuglN1+3W4UZI9htf04=lN6dwd|7NtP;^Orh}~VkyH9`Gy3P%{_384n_Vq10ephW4w7(zsD=~}z(fR#H z-uEA_tFr0p)!3H2Z^HAuNpByPYMV)$*=%#x-WAyMccbZ2J5uEeXwjUhdM9**5h4=&36^ zYwk6(y}jM7=kf*n?Yr5UxAr=$zTeX4n6diEdZtTJObVUz^n`5xI-4X(eVhKV*nUp= z*-CrfyKkfa?hW|knAp7P%l%dFXT9C;=*IG5?q`OBts-A|J~jTo$I4kR(E2Ux4!409 z+w>nS$2<~ldh$x0_tWxE*PncG&D&I-65-ShOB(B%rWu^RsqQ)N?>RBWjqh)!U-?tJ z-=+7!EaOEP#^+Ah9W0zu!!1+0jp68zFuT%6JkM5CwR)Vl_{vqgb;Zok`QGnLvr?BU zeJXqQ`SH<^IL9p(_JUiMTK;KDs;YmpWC2G~z@?0{>Kw~X83|+_^pxhE=cvWLMpng) z@7@G8p~I=Co}Mg9(4C|-*&y0Qwf0K4zlz<>6Sq{ejx5UBub{-zVk~}HdW*j5w^b*O z9&&wI67jnvPHK18sTtf#(JRk+&aM^=hlJ_|B=?tGM6btYuflpF7J9Je?{)hK=Z z0t|zrmda-(15G5;O;Vc!ME12FxtPw~(UbpFm?_=By;W3cM&ThuAiO%`=O}XK%=7l{ zl~Xq!v}kYbIaV*3x^Km-8wGxXW$|sw5f6m-MWuYP^zP2}TkLYS_pHuF-$?2CD(B4& zC$etde8RB5Mox6?o214!&x=2LRttTqn6|P&#+kvF<~>UL~8Rlg&nx!5UQ#-|*?@HI|uk;&lqdALyk=|30F5*fC#`Z+4C^@}@uKSU^P! z)F6R-@M|FLzQDH8*O0T5$T z$G~ulVR8bK?qnObO+aH+ycyJbfyUkfVrR%cJv^g2Osgt*Gyy8JN zZ1OD5B1Vph@#2$JxWd#$m=Pr@vUlYf6oA%3z?Md}$qTtuc#w=Dy_1vC7&OcW1$q%@>m>W1DO03vaCm-PE0LnP> oC^EHj0hP&2Zs0Kn3T)v~WcK0#DGQve$;-nvm6?Gdlo#X(0E2*hGynhq diff --git a/packages/scratch-vm/test/fixtures/corrupt_svg.sprite3 b/packages/scratch-vm/test/fixtures/corrupt_svg.sprite3 index 1d9dd0b3dd08da0ca2ebdfd1c435b295e44d179e..5be5353d9eb98f215732d26c81859d572c56e622 100644 GIT binary patch delta 497 zcmbO!dP_hgz?+#xgn@y9gTb<|C1n1`)^(;#8%0`~WMINE$%~@S|9JiXHc*i@BLjm9 zgABuD0XDUIP6p-&{URA4Tw1}+z{v8FnSlXJ`1bqyvncYky;u9!zD(SoZ;gCscEY!` zZ;#G?eURJ2rJ`e;Sa7gjpU2QJ^VdY1_qW4jPTf4*`smd&g+lWJ7Ot#odJ>(>w@<&V zZqsu;JZZ1Qii`-=TT`Z`A1gZNyifjF#tqr$b3*Hx6s_mxe0_NJ_#|VquM49XIE=eI zwEiysx8Pcx<$G1POBP($EG~zho}!b{x!#)3v~R=n5BDx@?3Ir#|NL_Q%2!f*RtimJ zS*q*!W8#m9d<)j6&!?I!Ims~f<(`UL5l%JMyjj^m=CJ_bCZM5{z&rqa$gThY delta 1080 zcmca5FjG`Ez?+#xgnZ6t}_LSf@BbS z&V?fMFtS1P7_zHRmS^Fvzd5%qskDNdfsy4aBLf4_5(b9ekh^)e6(sh~-%{@2Gt2Cx zzy{tnoxG*7Z?YzrZM1F3o_u3{KjcwJp z$d|32VYrRypHF1`t!2CGJ5OxArX7Fu#hs9C=L*k=xNXfW+uEAja_q&G8K0m3o0KfJ zk>k`vXGKA$*|&JuOqHd6?{$p5-^=#z#I;t|-P>G$Kf0&e82D|mZPg^#<%@E)SbpZs zInH-;;frIr_HT;cEYD3m%YG=>msQ-TkE8y%q^SII+r5jlavb@N9^^UVTm0ww-}lM+ z_NI@&FM6r7p~5$9W@Z)x2$XXdi!lg!>gJ6dJp#P zoUr>^NxkUJ?^7e4Z@1N6Y~yp_=WAUXIQ4OUjKTe7DqY5oM>`89UNxMoE+N0%`S8aj zIzP+0!}TSOaL(tgm=+MYspGJ+PwSPA46WpaB6qBm3@;=E9m)-uy40qR$wTC-l5a%> zQ|C_a*|ANW4_i1K15GX6Wf9v9qi!kDGL^|L@TyzitIj{W-@e zQ#HnK-jW{oNr#k*o$EPgOmfPef92_wIfe16m*gk0lnPkOoPFv#$JOywZT$4&%5N)g zdpW6B%u#$1ckIAv+oX;|jZc@W3YuKb;(8s(P_b&0ijHmf&Ya1?2A^2Jv}~5BG`JD7 z{a60Kvsvb~W`B1S#%D { const storedCostume = customCostume.asset; t.type(storedCostume, 'object'); - t.deepEquals(storedCostume.data, costumeData); + t.same(storedCostume.data, sanitizeByteStream(costumeData)); const sounds = vm.runtime.targets[1].sprite.sounds; t.equals(sounds.length, 1); diff --git a/packages/scratch-vm/test/integration/sb2_corrupted_svg.js b/packages/scratch-vm/test/integration/sb2_corrupted_svg.js index 5ad71fa17c..672a6cf1fb 100644 --- a/packages/scratch-vm/test/integration/sb2_corrupted_svg.js +++ b/packages/scratch-vm/test/integration/sb2_corrupted_svg.js @@ -15,6 +15,7 @@ const FakeBitmapAdapter = require('../fixtures/fake-bitmap-adapter'); const {extractAsset, readFileToBuffer} = require('../fixtures/readProjectFile'); const VirtualMachine = require('../../src/index'); const {serializeCostumes} = require('../../src/serialization/serialize-assets'); +const {sanitizeByteStream} = require('../../../scratch-svg-renderer/src/sanitize-svg'); const projectUri = path.resolve(__dirname, '../fixtures/corrupt_svg.sb2'); const project = readFileToBuffer(projectUri); @@ -23,7 +24,7 @@ const originalCostume = extractAsset(projectUri, costumeFileName); // We need to get the actual md5 because we hand modified the svg to corrupt it // after we downloaded the project from Scratch // Loading the project back into the VM will correct the assetId and md5 -const brokenCostumeMd5 = md5(originalCostume); +const brokenCostumeMd5 = md5(sanitizeByteStream(originalCostume)); global.Image = function () { const image = { @@ -57,7 +58,7 @@ tap.beforeEach(() => { // Mock renderer breaking on loading a corrupt costume FakeRenderer.prototype.createSVGSkin = function (svgString) { // Look for text added to costume to make it a corrupt svg - if (svgString.includes('')) { throw new Error('mock createSVGSkin broke'); } return FakeRenderer._nextSkinId++; diff --git a/packages/scratch-vm/test/integration/sb3_corrupted_svg.js b/packages/scratch-vm/test/integration/sb3_corrupted_svg.js index d4efe3653d..f8c8e4b268 100644 --- a/packages/scratch-vm/test/integration/sb3_corrupted_svg.js +++ b/packages/scratch-vm/test/integration/sb3_corrupted_svg.js @@ -14,6 +14,7 @@ const FakeRenderer = require('../fixtures/fake-renderer'); const {extractAsset, readFileToBuffer} = require('../fixtures/readProjectFile'); const VirtualMachine = require('../../src/index'); const {serializeCostumes} = require('../../src/serialization/serialize-assets'); +const {sanitizeByteStream} = require('../../../scratch-svg-renderer/src/sanitize-svg'); const projectUri = path.resolve(__dirname, '../fixtures/corrupt_svg.sb3'); const project = readFileToBuffer(projectUri); @@ -22,7 +23,7 @@ const originalCostume = extractAsset(projectUri, costumeFileName); // We need to get the actual md5 because we hand modified the svg to corrupt it // after we downloaded the project from Scratch // Loading the project back into the VM will correct the assetId and md5 -const brokenCostumeMd5 = md5(originalCostume); +const brokenCostumeMd5 = md5(sanitizeByteStream(originalCostume)); let vm; let defaultVectorAssetId; @@ -37,7 +38,7 @@ tap.beforeEach(() => { // Mock renderer breaking on loading a corrupt costume FakeRenderer.prototype.createSVGSkin = function (svgString) { // Look for text added to costume to make it a corrupt svg - if (svgString.includes('')) { throw new Error('mock createSVGSkin broke'); } return FakeRenderer._nextSkinId++; diff --git a/packages/scratch-vm/test/integration/sprite2_corrupted_svg.js b/packages/scratch-vm/test/integration/sprite2_corrupted_svg.js index 866448d797..b7776435d2 100644 --- a/packages/scratch-vm/test/integration/sprite2_corrupted_svg.js +++ b/packages/scratch-vm/test/integration/sprite2_corrupted_svg.js @@ -16,6 +16,7 @@ const FakeBitmapAdapter = require('../fixtures/fake-bitmap-adapter'); const {extractAsset, readFileToBuffer} = require('../fixtures/readProjectFile'); const VirtualMachine = require('../../src/index'); const {serializeCostumes} = require('../../src/serialization/serialize-assets'); +const {sanitizeByteStream} = require('../../../scratch-svg-renderer/src/sanitize-svg'); const projectUri = path.resolve(__dirname, '../fixtures/default.sb3'); const project = readFileToBuffer(projectUri); @@ -28,7 +29,7 @@ const originalCostume = extractAsset(spriteUri, costumeFileName); // We need to get the actual md5 because we hand modified the svg to corrupt it // after we downloaded the project from Scratch // Loading the project back into the VM will correct the assetId and md5 -const brokenCostumeMd5 = md5(originalCostume); +const brokenCostumeMd5 = md5(sanitizeByteStream(originalCostume)); global.Image = function () { const image = { @@ -61,7 +62,7 @@ tap.beforeEach(() => { // Mock renderer breaking on loading a corrupt costume FakeRenderer.prototype.createSVGSkin = function (svgString) { // Look for text added to costume to make it a corrupt svg - if (svgString.includes('')) { throw new Error('mock createSVGSkin broke'); } return FakeRenderer.prototype._nextSkinId++; diff --git a/packages/scratch-vm/test/integration/sprite3_corrupted_svg.js b/packages/scratch-vm/test/integration/sprite3_corrupted_svg.js index f78f468a06..6d2da371dd 100644 --- a/packages/scratch-vm/test/integration/sprite3_corrupted_svg.js +++ b/packages/scratch-vm/test/integration/sprite3_corrupted_svg.js @@ -15,6 +15,7 @@ const FakeRenderer = require('../fixtures/fake-renderer'); const {extractAsset, readFileToBuffer} = require('../fixtures/readProjectFile'); const VirtualMachine = require('../../src/index'); const {serializeCostumes} = require('../../src/serialization/serialize-assets'); +const {sanitizeByteStream} = require('../../../scratch-svg-renderer/src/sanitize-svg'); const projectUri = path.resolve(__dirname, '../fixtures/default.sb3'); const project = readFileToBuffer(projectUri); @@ -27,7 +28,7 @@ const originalCostume = extractAsset(spriteUri, costumeFileName); // We need to get the actual md5 because we hand modified the svg to corrupt it // after we downloaded the project from Scratch // Loading the project back into the VM will correct the assetId and md5 -const brokenCostumeMd5 = md5(originalCostume); +const brokenCostumeMd5 = md5(sanitizeByteStream(originalCostume)); let vm; let defaultVectorAssetId; @@ -42,7 +43,7 @@ tap.beforeEach(() => { // Mock renderer breaking on loading a corrupt costume FakeRenderer.prototype.createSVGSkin = function (svgString) { // Look for text added to costume to make it a corrupt svg - if (svgString.includes('')) { throw new Error('mock createSVGSkin broke'); } return FakeRenderer.prototype._nextSkinId++; From 7247ee7e9db2fb05498a3d2e6ead814dddd06901 Mon Sep 17 00:00:00 2001 From: MiroslavDionisiev Date: Fri, 9 May 2025 14:32:18 +0300 Subject: [PATCH 3/4] fix: logs for testing --- .../scratch-svg-renderer/src/sanitize-svg.js | 49 +++++++++++++++++-- .../scratch-vm/src/import/load-costume.js | 6 +++ packages/scratch-vm/src/serialization/sb2.js | 4 ++ packages/scratch-vm/src/serialization/sb3.js | 6 ++- 4 files changed, 60 insertions(+), 5 deletions(-) diff --git a/packages/scratch-svg-renderer/src/sanitize-svg.js b/packages/scratch-svg-renderer/src/sanitize-svg.js index fd88a60ecc..a5686f7b39 100644 --- a/packages/scratch-svg-renderer/src/sanitize-svg.js +++ b/packages/scratch-svg-renderer/src/sanitize-svg.js @@ -8,6 +8,8 @@ const DOMPurify = require('isomorphic-dompurify'); const sanitizeSvg = {}; +const isInternalRef = ref => ref.startsWith('#') || ref.startsWith('data:'); + DOMPurify.addHook( 'beforeSanitizeAttributes', currentNode => { @@ -15,8 +17,8 @@ DOMPurify.addHook( if (currentNode && currentNode.href && currentNode.href.baseVal) { const href = currentNode.href.baseVal.replace(/\s/g, ''); // "data:" and "#" are valid hrefs - if ((href.slice(0, 5) !== 'data:') && (href.slice(0, 1) !== '#')) { - + if (!isInternalRef(href)) { + // TODO: Those can be in different namespaces than `xlink:` if (currentNode.attributes.getNamedItem('xlink:href')) { currentNode.attributes.removeNamedItem('xlink:href'); delete currentNode['xlink:href']; @@ -27,6 +29,24 @@ DOMPurify.addHook( } } } + + // Remove url(...) usages with external references + if (currentNode && currentNode.attributes) { + for (let i = currentNode.attributes.length - 1; i >= 0; i--) { + const attr = currentNode.attributes[i]; + const rawValue = attr.value || ''; + const value = rawValue.toLowerCase().replace(/\s/g, ''); + + const urlMatch = value.match(/url\((.+?)\)/); + if (urlMatch) { + const ref = urlMatch[1].replace(/['"]/g, ''); + if (!isInternalRef(ref)) { + currentNode.removeAttribute(attr.name); + } + } + } + } + return currentNode; } ); @@ -37,13 +57,34 @@ DOMPurify.addHook( if (data.tagName === 'style') { const ast = parse(node.textContent); let isModified = false; - // Remove any @import rules as it could leak HTTP requests + walk(ast, (astNode, item, list) => { - if (astNode.type === 'Atrule' && astNode.name === 'import') { + // @import rules + if (astNode.type === 'Atrule' && astNode.name.toLowerCase() === 'import') { list.remove(item); isModified = true; } + + // Elements using url(...) for external resources + if (astNode.type === 'Declaration' && astNode.value) { + let shouldRemove = false; + walk(astNode.value, valueNode => { + if (valueNode.type === 'Url') { + const urlValue = (valueNode.value.value || '').trim().replace(/['"]/g, ''); + + if (!isInternalRef(urlValue)) { + shouldRemove = true; + } + } + }); + + if (shouldRemove) { + list.remove(item); + isModified = true; + } + } }); + if (isModified) { node.textContent = generate(ast); } diff --git a/packages/scratch-vm/src/import/load-costume.js b/packages/scratch-vm/src/import/load-costume.js index e430ad3157..492a55777a 100644 --- a/packages/scratch-vm/src/import/load-costume.js +++ b/packages/scratch-vm/src/import/load-costume.js @@ -356,6 +356,8 @@ const loadCostumeFromAsset = function (costume, runtime, optVersion) { * @returns {?Promise} - a promise which will resolve after skinId is set, or null on error. */ const loadCostume = function (md5ext, costume, runtime, optVersion) { + console.log('CCCCCCCCCCCCCCCCCCCCCCCCCCCC'); + console.log(md5ext); const idParts = StringUtil.splitFirst(md5ext, '.'); const md5 = idParts[0]; const ext = idParts[1].toLowerCase(); @@ -394,6 +396,8 @@ const loadCostume = function (md5ext, costume, runtime, optVersion) { if (assetArray[0]) { costume.asset = assetArray[0]; } else { + console.log('BBBBBBBBBBBBBBBBBBBBBBBBBBB'); + console.log(md5ext); return handleCostumeLoadError(costume, runtime); } @@ -405,6 +409,8 @@ const loadCostume = function (md5ext, costume, runtime, optVersion) { .catch(error => { // Handle case where storage.load rejects with errors // instead of resolving null + console.log('AAAAAAAAAAAAAAAAAAAAAAAA'); + console.log(md5ext); log.warn('Error loading costume: ', error); return handleCostumeLoadError(costume, runtime); }); diff --git a/packages/scratch-vm/src/serialization/sb2.js b/packages/scratch-vm/src/serialization/sb2.js index 855723d6a4..63c3a69b85 100644 --- a/packages/scratch-vm/src/serialization/sb2.js +++ b/packages/scratch-vm/src/serialization/sb2.js @@ -452,6 +452,10 @@ const parseScratchAssets = function (object, runtime, topLevel, zip) { if (costumeSource.textLayerMD5) { costume.textLayerMD5 = StringUtil.splitFirst(costumeSource.textLayerMD5, '.')[0]; } + console.log('////////////////////////////'); + console.log('sb2: old md5'); + console.log(costume.md5); + console.log('////////////////////////////'); // If there is no internet connection, or if the asset is not in storage // for some reason, and we are doing a local .sb2 import, (e.g. zip is provided) // the file name of the costume should be the baseLayerID followed by the file ext diff --git a/packages/scratch-vm/src/serialization/sb3.js b/packages/scratch-vm/src/serialization/sb3.js index c7db58d0b3..6d9f1dc62e 100644 --- a/packages/scratch-vm/src/serialization/sb3.js +++ b/packages/scratch-vm/src/serialization/sb3.js @@ -894,13 +894,17 @@ const parseScratchAssets = function (object, runtime, zip) { costumeSource.md5ext : `${costumeSource.assetId}.${dataFormat}`; costume.md5 = costumeMd5Ext; costume.dataFormat = dataFormat; + console.log('////////////////////////////'); + console.log('sb3: old md5'); + console.log(costumeMd5Ext); + console.log('////////////////////////////'); // deserializeCostume should be called on the costume object we're // creating above instead of the source costume object, because this way // we're always loading the 'sb3' representation of the costume // any translation that needs to happen will happen in the process // of building up the costume object into an sb3 format return deserializeCostume(costume, runtime, zip) - .then(() => loadCostume(costumeMd5Ext, costume, runtime)); + .then(() => loadCostume(costume.md5, costume, runtime)); // Only attempt to load the costume after the deserialization // process has been completed }); From 8113d9c3de2502ee8045ad59ca78c425b7ad1743 Mon Sep 17 00:00:00 2001 From: MiroslavDionisiev Date: Thu, 22 May 2025 10:35:09 +0300 Subject: [PATCH 4/4] Revert "fix: logs for testing" This reverts commit 7247ee7e9db2fb05498a3d2e6ead814dddd06901. --- .../scratch-svg-renderer/src/sanitize-svg.js | 49 ++----------------- .../scratch-vm/src/import/load-costume.js | 6 --- packages/scratch-vm/src/serialization/sb2.js | 4 -- packages/scratch-vm/src/serialization/sb3.js | 6 +-- 4 files changed, 5 insertions(+), 60 deletions(-) diff --git a/packages/scratch-svg-renderer/src/sanitize-svg.js b/packages/scratch-svg-renderer/src/sanitize-svg.js index a5686f7b39..fd88a60ecc 100644 --- a/packages/scratch-svg-renderer/src/sanitize-svg.js +++ b/packages/scratch-svg-renderer/src/sanitize-svg.js @@ -8,8 +8,6 @@ const DOMPurify = require('isomorphic-dompurify'); const sanitizeSvg = {}; -const isInternalRef = ref => ref.startsWith('#') || ref.startsWith('data:'); - DOMPurify.addHook( 'beforeSanitizeAttributes', currentNode => { @@ -17,8 +15,8 @@ DOMPurify.addHook( if (currentNode && currentNode.href && currentNode.href.baseVal) { const href = currentNode.href.baseVal.replace(/\s/g, ''); // "data:" and "#" are valid hrefs - if (!isInternalRef(href)) { - // TODO: Those can be in different namespaces than `xlink:` + if ((href.slice(0, 5) !== 'data:') && (href.slice(0, 1) !== '#')) { + if (currentNode.attributes.getNamedItem('xlink:href')) { currentNode.attributes.removeNamedItem('xlink:href'); delete currentNode['xlink:href']; @@ -29,24 +27,6 @@ DOMPurify.addHook( } } } - - // Remove url(...) usages with external references - if (currentNode && currentNode.attributes) { - for (let i = currentNode.attributes.length - 1; i >= 0; i--) { - const attr = currentNode.attributes[i]; - const rawValue = attr.value || ''; - const value = rawValue.toLowerCase().replace(/\s/g, ''); - - const urlMatch = value.match(/url\((.+?)\)/); - if (urlMatch) { - const ref = urlMatch[1].replace(/['"]/g, ''); - if (!isInternalRef(ref)) { - currentNode.removeAttribute(attr.name); - } - } - } - } - return currentNode; } ); @@ -57,34 +37,13 @@ DOMPurify.addHook( if (data.tagName === 'style') { const ast = parse(node.textContent); let isModified = false; - + // Remove any @import rules as it could leak HTTP requests walk(ast, (astNode, item, list) => { - // @import rules - if (astNode.type === 'Atrule' && astNode.name.toLowerCase() === 'import') { + if (astNode.type === 'Atrule' && astNode.name === 'import') { list.remove(item); isModified = true; } - - // Elements using url(...) for external resources - if (astNode.type === 'Declaration' && astNode.value) { - let shouldRemove = false; - walk(astNode.value, valueNode => { - if (valueNode.type === 'Url') { - const urlValue = (valueNode.value.value || '').trim().replace(/['"]/g, ''); - - if (!isInternalRef(urlValue)) { - shouldRemove = true; - } - } - }); - - if (shouldRemove) { - list.remove(item); - isModified = true; - } - } }); - if (isModified) { node.textContent = generate(ast); } diff --git a/packages/scratch-vm/src/import/load-costume.js b/packages/scratch-vm/src/import/load-costume.js index 492a55777a..e430ad3157 100644 --- a/packages/scratch-vm/src/import/load-costume.js +++ b/packages/scratch-vm/src/import/load-costume.js @@ -356,8 +356,6 @@ const loadCostumeFromAsset = function (costume, runtime, optVersion) { * @returns {?Promise} - a promise which will resolve after skinId is set, or null on error. */ const loadCostume = function (md5ext, costume, runtime, optVersion) { - console.log('CCCCCCCCCCCCCCCCCCCCCCCCCCCC'); - console.log(md5ext); const idParts = StringUtil.splitFirst(md5ext, '.'); const md5 = idParts[0]; const ext = idParts[1].toLowerCase(); @@ -396,8 +394,6 @@ const loadCostume = function (md5ext, costume, runtime, optVersion) { if (assetArray[0]) { costume.asset = assetArray[0]; } else { - console.log('BBBBBBBBBBBBBBBBBBBBBBBBBBB'); - console.log(md5ext); return handleCostumeLoadError(costume, runtime); } @@ -409,8 +405,6 @@ const loadCostume = function (md5ext, costume, runtime, optVersion) { .catch(error => { // Handle case where storage.load rejects with errors // instead of resolving null - console.log('AAAAAAAAAAAAAAAAAAAAAAAA'); - console.log(md5ext); log.warn('Error loading costume: ', error); return handleCostumeLoadError(costume, runtime); }); diff --git a/packages/scratch-vm/src/serialization/sb2.js b/packages/scratch-vm/src/serialization/sb2.js index 63c3a69b85..855723d6a4 100644 --- a/packages/scratch-vm/src/serialization/sb2.js +++ b/packages/scratch-vm/src/serialization/sb2.js @@ -452,10 +452,6 @@ const parseScratchAssets = function (object, runtime, topLevel, zip) { if (costumeSource.textLayerMD5) { costume.textLayerMD5 = StringUtil.splitFirst(costumeSource.textLayerMD5, '.')[0]; } - console.log('////////////////////////////'); - console.log('sb2: old md5'); - console.log(costume.md5); - console.log('////////////////////////////'); // If there is no internet connection, or if the asset is not in storage // for some reason, and we are doing a local .sb2 import, (e.g. zip is provided) // the file name of the costume should be the baseLayerID followed by the file ext diff --git a/packages/scratch-vm/src/serialization/sb3.js b/packages/scratch-vm/src/serialization/sb3.js index 6d9f1dc62e..c7db58d0b3 100644 --- a/packages/scratch-vm/src/serialization/sb3.js +++ b/packages/scratch-vm/src/serialization/sb3.js @@ -894,17 +894,13 @@ const parseScratchAssets = function (object, runtime, zip) { costumeSource.md5ext : `${costumeSource.assetId}.${dataFormat}`; costume.md5 = costumeMd5Ext; costume.dataFormat = dataFormat; - console.log('////////////////////////////'); - console.log('sb3: old md5'); - console.log(costumeMd5Ext); - console.log('////////////////////////////'); // deserializeCostume should be called on the costume object we're // creating above instead of the source costume object, because this way // we're always loading the 'sb3' representation of the costume // any translation that needs to happen will happen in the process // of building up the costume object into an sb3 format return deserializeCostume(costume, runtime, zip) - .then(() => loadCostume(costume.md5, costume, runtime)); + .then(() => loadCostume(costumeMd5Ext, costume, runtime)); // Only attempt to load the costume after the deserialization // process has been completed });