phpseclib: $sftp->delete(null); doesn't raise an error, but deletes local folders

During development, I had a typo in a variable name, which resulted in me sending in null to the SFTP::delete() method. Instead of return an error, I noticed that the script (which ran through wp-cli on an Ubuntu box) took an unusually long time.

When I tried again, I got unknown command wp. The reason was that the SFTP::delete() method had recursively deleted everything in my home folder. My wp-cli bin file was in a sub folder in my home folder.

If course, you should always check what you send in, but in case you do something wrong during development on your local machine, this could be quite bad. I was lucky, since I’m running it in a virtual machine and recently made a snapshot.

Note: In my case, I SFTP’d to the same machine, with the same user I was running the script as (as a mock), but it’s still an unexpected behavior.

About this issue

Original URL
State: closed
Created 8 years ago
Comments: 16 (8 by maintainers)

Commits related to this issue

SFTP: update directory deletion check (#1059) — committed to phpseclib/phpseclib by terrafrost 8 years ago

Most upvoted comments

Don’t think it’s necessary to add the overhead of looking for the __toString()-method. Since PHP will throw a fatal error: “Object of class foo could not be converted to string” if it doesn’t exist, it might actually help developers debugging their code, in those situations.

That’s true. Altho at the same time it seems like if we’re gonna go down that path we could also do user_error('The path that's been passed is not a valid string') when people pass in null’s or int’s or whatever. idk. I don’t really have any personal preference either way lol.

The documentation should be updated, though, so it says that recursive deletes are the default behavior.

Yah - that’s been done.

terrafrost on Nov 20, 2016