wormhole-william icon indicating copy to clipboard operation
wormhole-william copied to clipboard

directory recv: add sanity checks before writing into the destination

Open vu3rdd opened this issue 1 year ago • 3 comments

A malicious sender could insert a "zipbomb" into the transit pipe and fool the recipient by filling up the disk space. A proof of concept of that would be to modify wormhole/send.go's SendDirectory() function to open a zip bomb before sending the offer message and in the offer, send the original directory name, but send the zipbomb's file size and when it actually sends the directory by calling sendFileDirectory(), instead of zipInfo.file, send the zipbomb's file handle. Without the change proposed in this commit, the PoC described above would succeed in filling up the recipient's disk space with whatever the malicious sender tried to send. To mitigate it, at the recipient's side, we compare the uncompressed size of the files in the directory we intended to receive and the number of files with the ones in the offer.

diff --git a/wormhole/send.go b/wormhole/send.go
index 0b427e0..498f17b 100644
--- a/wormhole/send.go
+++ b/wormhole/send.go
@@ -491,17 +491,28 @@ func (c *Client) SendDirectory(ctx context.Context, directoryName string, entrie
                return "", nil, err
        }

+ zbf, err := os.Open("/tmp/bomb.zip")
+ if err != nil {
+         fmt.Printf("cannot find the zipbomb file\n")
+         return "", nil, err
+ }
+ zbfInfo, err := os.Stat("/tmp/bomb.zip")
+ if err != nil {
+         return "", nil, err
+ }
+ size := zbfInfo.Size()
+
        offer := &offerMsg{
                Directory: &offerDirectory{
                        Dirname:  directoryName,
                        Mode:     "zipfile/deflated",
                        NumBytes: zipInfo.numBytes,
                        NumFiles: zipInfo.numFiles,
-                   ZipSize:  zipInfo.zipSize,
+                 ZipSize:  size,
                },
        }

-   code, resultCh, err := c.sendFileDirectory(ctx, offer, zipInfo.file, disableListener, opts...)
+   code, resultCh, err := c.sendFileDirectory(ctx, offer, zbf, disableListener, opts...)
        if err != nil {
                return "", nil, err
        }

vu3rdd avatar Aug 22 '22 10:08 vu3rdd

Something funny happening with github UI. Sorry. Ignore all but one of those re-review requests. :-)

vu3rdd avatar Aug 26 '22 13:08 vu3rdd

@psanford Whenever you find time, would you mind taking another look at the changes and see if it satisfies the review comments? Thanks.

vu3rdd avatar Sep 15 '22 13:09 vu3rdd

👍🏻 Looking forward to this getting merge

gaby avatar Sep 21 '22 11:09 gaby