Herschel / Swivel

Adobe Flash SWF to video converter
https://newgrounds.com/swivel
GNU General Public License v3.0
165 stars 22 forks source link

DEFINESHAPE (etc) blocks in modified .swfs are wrong size #7

Open andrew-klaassen opened 6 years ago

andrew-klaassen commented 6 years ago

A couple of my swfs are fine in the 2013 release of Swivel, but always crash the current version. By turning off all mutations in both 2013 and current versions, and then writing out the intermediate swf that gets fed to _loader.loadBytes(), I think I'm getting closer to finding the culprit: The byte length of some blocks is changed by a few bytes in the intermediate swf, but only using the current version. The intermediate swf created by the 2013 version doesn't have these differences.

Using swfdump and diff, here are a couple of typical lines showing the difference between the original swf and the (theoretically unmutated) intermediate swf created by the current version of Swivel:

40958c40958
< [020]      3925 DEFINESHAPE3 defines id 0048
---
> [020]      3927 DEFINESHAPE3 defines id 0048
41994c41994
< [02e]       442 DEFINEMORPHSHAPE defines id 0051
---
> [02e]       441 DEFINEMORPHSHAPE defines id 0051

In the swf I'm testing:

DEFINESHAPE2, DEFINESHAPE3: ~25% of blocks are 1-65 bytes larger than the original DEFINESHAPE: ~5% of blocks are 1 byte larger than the original DEFINEMORPHSHAPE: 100% of blocks are 1 byte smaller than the original PLACEOBJECT2: ~.005% of blocks are 6 bytes smaller than the original

Everything else in a detailed dump (swfdump -D) comes out the same.

I'll hopefully get a chance to look into this further sometime in the next few days. I wanted to get it written down before I forget what I've figured out.

Herschel commented 6 years ago

If possible, could you attach a sample FLA and/or SWF?

The current fork of the format lib is most likely parsing or writing some aspect of the shape data incorrectly compared to my own shape parsing code in the 2013 release.

andrew-klaassen commented 6 years ago

I'll have to get corporate approval for that, since it's corporate IP - let me get back to you once I've had a chance to ask. (I may not get an answer until later next week... or after Christmas...) I'll try to do some digging myself in the meantime, since I'd like to continue wrapping my head around more of this.

andrew-klaassen commented 6 years ago

Correction: The PLACEOBJECT2 blocks which are different are 1 byte larger than the original.

I'm not sure at this point whether the difference is in the actual size of the blocks or only in the reported size.

Herschel commented 6 years ago

Thanks! If you can repro the problem in a minimal example SWF, that helps too!

andrew-klaassen commented 6 years ago

I've attached a minimal example which shows a couple of the changes that the current version makes to the SWF with all mutators turned off. It doesn't make Flash Player go boom like the production SWF does, but hopefully it illustrates some of the mangling that's happening.

movecircle.zip

movecircle.: Original files. modified-2013.: Intermediate SWF created by Newgrounds 2013 release of Swivel. modified-2017.*: Intermediate SWF created by current Swivel.

I've pushed compiled Swivel versions to my write-intermediate branch if you want to play with them:

https://github.com/andrew-klaassen/Swivel/tree/origin/write-intermediate/bin

They go into an endless loop when they get to the movie-outputting stage, but just before that they put a "modified-201[37].swf" file into the application storage directory (...\com.newgrounds.swivel.Swivel\Local Store).

For now, I've been able to hack your missing-first-frame fix onto the 2013 release, and it seems to be working fine.

Herschel commented 6 years ago

I took a quick look, and I think the attached SWF is working fine; there's some flexibility in the way you can write data in the SWF file, so you can have SWFs with different data, but that are semantically the same.

In particular, the difference in the dumps you posted is the matrix of the last PlaceObject2 tag, which has an extra byte. The transform has zero translation, which is being written with a length of 1 bit in the current format version, but my old code didn't bother to write the translation in this case and wrote it with a length of 0. Both should be equivalent and valid, so I don't think this is the issue you're hitting in the problem SWF!

andrew-klaassen commented 6 years ago

Ah, that makes sense. I was just stepping through the matrix code, trying to make sense of it; thanks for the explanation.

I'll keep working on trying to get a broken version to send you.

Herschel commented 6 years ago

No worries, thanks for helping out!!!

andrew-klaassen commented 6 years ago

This might be less than helpful in the absence of the full file, but I think that I've narrowed the problem down to the DEFINEMORPHSHAPE changes. I've created two SWFs where this is the only difference:

Working (original, differences with non-working highlighted with **):

[02e]       **442** DEFINEMORPHSHAPE defines id 0213
                -=> d5 00 74 db ad 10 ee f2 d9 f8 74 db ad 10 ee f2     Õ.tÛ­.îòÙøtÛ­.îò
                -=> d9 f8 **f2** 00 00 00 01 **12** c5 17 18 8a 3c bd 8d 27     Ùøò.....Å..Š<½'
                -=> 37 76 52 78 ed 00 c5 17 18 8a 3c bd 8d 27 37 76     7vRxí.Å..Š<½'7v
                -=> 52 78 ed 00 **02** 00 38 c9 ec ff 00 3e ec 58 ff ff     Rxí...8Éìÿ.>ìXÿÿ
                -=> 21 55 83 ff ff 3c 70 3b ff 01 00 00 00 00 00 00     !Uƒÿÿ<p;ÿ.......
                -=> 00 00 00 00 00 00 11 2d d3 6f a8 81 d1 fc 53 a0     .......-Óo¨ÑüS 
                -=> dd 2f 50 37 d1 b1 32 d8 85 26 36 67 a6 4a 47 52     Ý/P7ѱ2Ø…&6g¦JGR
                -=> 1c d7 8a aa 6d cc ea cd ba cf 83 51 c9 75 fa 08     .׊ªmÌêͺσQÉuú.
                -=> ff 6e b7 1f f6 a7 01 6d 44 02 a5 ee 01 d6 da 4d     ÿn·.ö§.mD.¥î.ÖÚM
                -=> 27 59 0f d5 a4 9a 95 5c 91 d0 e5 43 e9 4c 6b 74     'Y.Õ¤š•\‘ÐåCéLkt
                -=> c6 b3 1a 89 e9 47 c1 81 86 68 04 93 1f f1 28 a3     Ƴ.‰éGÁ†h.“.ñ(£
                -=> fc 42 3e a4 03 5f a4 93 7a 22 6f 9e 6d c7 4b db     üB>¤._¤“z"ožmÇKÛ
                -=> 60 93 57 4b 88 f5 f3 07 2f 19 c4 50 e3 54 07 75     `“WKˆõó./.ÄPãT.u
                -=> e4 0c 24 16 9f 00 35 32 4f ff ce 4f af 54 e2 7d     ä.$.Ÿ.52OÿÎO¯Tâ}
                -=> 8d 86 78 c7 19 cf b0 a2 a0 ec ae ee b2 9b c3 d6     †xÇ.Ï°¢ ì®î²›ÃÖ
                -=> f8 8f 43 96 55 6b 15 35 d2 87 36 ee 27 ad e6 bf     øC–Uk.5Ò‡6î'­æ¿
                -=> 77 28 f9 ef 8f ee f5 00 **00** 05 d3 6f a8 81 47 f1     w(ùïîõ...Óo¨Gñ
                -=> 4e 83 74 bd 40 df 46 c4 cb 62 14 98 d9 9e 99 29     Nƒt½@ßFÄËb.˜Ùž™)
                -=> 1d 48 73 5e 2a a9 b7 33 ab 36 eb 3e 0d 47 25 d7     .Hs^*©·3«6ë>.G%×
                -=> e8 23 fd ba dc 7f da 9c 05 b5 10 0a 97 b8 07 5b     è#ýºÜ.Úœ.µ..—¸.[
                -=> 69 34 9d 64 3f 56 92 6a 55 72 47 43 95 0f a5 31     i4d?V’jUrGC•.¥1
                -=> ad d3 1a cc 6a 27 a5 1f 06 06 19 a0 12 4c 7f c4     ­Ó.Ìj'¥.... .L.Ä
                -=> a2 8f f1 08 fa 90 0d 7e 92 4d e8 89 be 79 b7 1d     ¢ñ.ú.~’M艾y·.
                -=> 2f 6d 82 4d 5d 2e 23 d7 cc 1c bc 67 11 43 8d 50     /m‚M].#×Ì.¼g.CP
                -=> 1d d7 90 30 90 5a 7c 00 d4 c9 3f ff 39 3e bd 53     .א0Z|.ÔÉ?ÿ9>½S
                -=> 89 f6 36 19 e3 1c 67 3e c2 8a 83 b2 bb ba ca 6f     ‰ö6.ã.g>Šƒ²»ºÊo
                -=> 0f 5b e2 3d 0e 59 55 ac 54 d7 4a 1c db b8 9e b7     .[â=.YU¬T×J.Û¸ž·
                -=> 9a fd dc a3 e7 be 3f bb d4 00                       šýÜ£ç¾?»Ô.

Not working (produced by Git version of Swivel):

[02e]       **441** DEFINEMORPHSHAPE defines id 0213
                -=> d5 00 74 db ad 10 ee f2 d9 f8 74 db ad 10 ee f2     Õ.tÛ­.îòÙøtÛ­.îò
                -=> d9 f8 **f1** 00 00 00 01 **10** c5 17 18 8a 3c bd 8d 27     Ùøñ.....Å..Š<½'
                -=> 37 76 52 78 ed 00 c5 17 18 8a 3c bd 8d 27 37 76     7vRxí.Å..Š<½'7v
                -=> 52 78 ed 00 00 38 c9 ec ff 00 3e ec 58 ff ff 21     Rxí..8Éìÿ.>ìXÿÿ!
                -=> 55 83 ff ff 3c 70 3b ff 01 00 00 00 00 00 00 00     Uƒÿÿ<p;ÿ........
                -=> 00 00 00 00 00 11 2d d3 6f a8 81 d1 fc 53 a0 dd     ......-Óo¨ÑüS Ý
                -=> 2f 50 37 d1 b1 32 d8 85 26 36 67 a6 4a 47 52 1c     /P7ѱ2Ø…&6g¦JGR.
                -=> d7 8a aa 6d cc ea cd ba cf 83 51 c9 75 fa 08 ff     ׊ªmÌêͺσQÉuú.ÿ
                -=> 6e b7 1f f6 a7 01 6d 44 02 a5 ee 01 d6 da 4d 27     n·.ö§.mD.¥î.ÖÚM'
                -=> 59 0f d5 a4 9a 95 5c 91 d0 e5 43 e9 4c 6b 74 c6     Y.Õ¤š•\‘ÐåCéLktÆ
                -=> b3 1a 89 e9 47 c1 81 86 68 04 93 1f f1 28 a3 fc     ³.‰éGÁ†h.“.ñ(£ü
                -=> 42 3e a4 03 5f a4 93 7a 22 6f 9e 6d c7 4b db 60     B>¤._¤“z"ožmÇKÛ`
                -=> 93 57 4b 88 f5 f3 07 2f 19 c4 50 e3 54 07 75 e4     “WKˆõó./.ÄPãT.uä
                -=> 0c 24 16 9f 00 35 32 4f ff ce 4f af 54 e2 7d 8d     .$.Ÿ.52OÿÎO¯Tâ}
                -=> 86 78 c7 19 cf b0 a2 a0 ec ae ee b2 9b c3 d6 f8     †xÇ.Ï°¢ ì®î²›ÃÖø
                -=> 8f 43 96 55 6b 15 35 d2 87 36 ee 27 ad e6 bf 77     C–Uk.5Ò‡6î'­æ¿w
                -=> 28 f9 ef 8f ee f5 00 **11** 05 d3 6f a8 81 47 f1 4e     (ùïîõ...Óo¨GñN
                -=> 83 74 bd 40 df 46 c4 cb 62 14 98 d9 9e 99 29 1d     ƒt½@ßFÄËb.˜Ùž™).
                -=> 48 73 5e 2a a9 b7 33 ab 36 eb 3e 0d 47 25 d7 e8     Hs^*©·3«6ë>.G%×è
                -=> 23 fd ba dc 7f da 9c 05 b5 10 0a 97 b8 07 5b 69     #ýºÜ.Úœ.µ..—¸.[i
                -=> 34 9d 64 3f 56 92 6a 55 72 47 43 95 0f a5 31 ad     4d?V’jUrGC•.¥1­
                -=> d3 1a cc 6a 27 a5 1f 06 06 19 a0 12 4c 7f c4 a2     Ó.Ìj'¥.... .L.Ä¢
                -=> 8f f1 08 fa 90 0d 7e 92 4d e8 89 be 79 b7 1d 2f     ñ.ú.~’M艾y·./
                -=> 6d 82 4d 5d 2e 23 d7 cc 1c bc 67 11 43 8d 50 1d     m‚M].#×Ì.¼g.CP.
                -=> d7 90 30 90 5a 7c 00 d4 c9 3f ff 39 3e bd 53 89     א0Z|.ÔÉ?ÿ9>½S‰
                -=> f6 36 19 e3 1c 67 3e c2 8a 83 b2 bb ba ca 6f 0f     ö6.ã.g>Šƒ²»ºÊo.
                -=> 5b e2 3d 0e 59 55 ac 54 d7 4a 1c db b8 9e b7 9a     [â=.YU¬T×J.Û¸ž·š
                -=> fd dc a3 e7 be 3f bb d4 00                          ýÜ£ç¾?»Ô.

The PLACEOBJECT2 which immediately follows is fine, but the second time that id 0213 is accessed - for another PLACEOBJECT2 17 frames later - Flash Player (and Swivel) crash.

Herschel commented 6 years ago

Yikes, it looks like the morph shape writing code in the current format lib is a mess. I see quite a few problems in there, chiefly I think the # of colors in the gradient is not being written. Also looks like it's writing the radial gradient as linear gradient there. There's also #5, so I think I should just check over all of the shape tween code in format.

Relevant code: https://github.com/Herschel/format/blob/19424528ba01fdf67a9482055cc33d6571fe70c3/format/swf/Writer.hx#L868-L911 SWF specs: http://tilde.town/~herschel/swf-file-format-spec.pdf p149 MORPHGRADIENT

I can fix this later tonight, or if you want to give it a shot, let me know!

Herschel commented 6 years ago

(I actually think the old Swivel didn't even bother to parse the DefineMorphShape tags and just read the bytes as is. But since it's in format now, might as well fix it.)

andrew-klaassen commented 6 years ago

I just now pieced my way through the bits to get to the part where swf_gradient structure is missing its count. I'm happy that you came to the same conclusion, because it means that all the manual bit-aligning I've been doing for the past couple of hours wasn't a waste. :-)

I am about to switch into child care mode, but I wouldn't mind giving it an attempt tomorrow. If you know of any magical tools which break down SWF bit fields so that I don't have to do it bit-by-bit myself, I would love to know about them! (I did learn a lot by going bit-by-bit, but it was a bit painful.)

Herschel commented 6 years ago

I think the best way is to use the HaxeDevelop debugger and put a breakpoint in format/swf/Reader.hx or Writer.hx. You can step thru and see where things get weird.

Also, this is the best SWF decompiler and inspector around: https://www.free-decompiler.com/flash/

andrew-klaassen commented 6 years ago

I've been stepping through with the HaxeDevelop debugger, but for this problem it wasn't as helpful since Swivel would just go boom when it called _frame.drawWithQuality(), and the debugger would just sit there silently. I ended up using a manual binary search, stitching together part of the original SWF with part of the mangled SWF until I found the specific location of the problem. Very hacky, but it got me there eventually. Stepping through with the debugger definitely did give me a better understanding of the program flow, though.

I will check out that decompiler - thanks!

andrew-klaassen commented 6 years ago

Oh, wait, I have been using that decompiler! With the broken SWF, the morph shapes were simply missing, which probably should've given me a clue as to where to look.

andrew-klaassen commented 6 years ago

I've submitted a couple of pull requests for format.swf:

https://github.com/andrew-klaassen/format/commit/00c26ea8b823d144e23b39a09c5a912da873a6e8 fixes the crash that led to this bug report.

https://github.com/andrew-klaassen/format/commit/2b58581ed775f79b799a490b62cddcdfd24d4787 matches the matrix writing behaviour of Flash so that real bugs are easier to spot.

The other two commits in my pull requests deal with other things; should I open new issues for them? (I haven't done much on Github so far, as you can see, so I'm uncertain about the niceties. :-) )

Thanks.