Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SFTP Stream Wrapper doesn't handle url with special characters #1779

Open
michael-lavaveshkul opened this issue Apr 14, 2022 · 5 comments
Open

Comments

@michael-lavaveshkul
Copy link

https://github.com/phpseclib/phpseclib/blob/3.0.14/phpseclib/Net/SFTP/Stream.php#L158

parse_url does not decode the values extracted from the url.

This function parses a URL and returns an associative array containing any of the various components of the URL that are present. The values of the array elements are not URL decoded.
https://www.php.net/manual/en/function.parse-url.php

This can lead to things like $this->sftp->login($user, $pass) failing to login because it has the wrong username/password. Or $this->sftp->filesize($path); failing because the $path doesn't match what's actually on the server.

This problem can be avoided by using the SFTP object directly.

Alternatively, setting the username and password via the stream context can fix the issue with special characters in the credentials, but I didn't see a workaround if you have a file path with characters that could be url encoded.

@terrafrost
Copy link
Member

I guess my most immediate thoughts are: what does libssh2 do?

If you have a username or password with % in it... do you need to pass % to libssh2 or do you need to pass %25 to it?

Even if libssh2 doesn't, itself, urldecode special characters, I suppose it's also fair to ask whether or not it ought to be.

I got stuff to do today but I'll try to take a look at this this evening or in the next few days.

Thanks!

@terrafrost
Copy link
Member

So I tested this out by creating an account test with a password of te%st. ssh2_auth_password($ssh, 'test', 'te%st') worked, ssh2_auth_password($ssh, 'test', 'te%25st') did not. This suggests that https://www.php.net/ssh2 does not behave as you're suggesting phpseclib ought to.

I guess this would be a problem if your username had a : in it. Superficially it seems like it ought to be an issue if your username was user@domain.tld but that does not appear to be the case.

Here's what I tried:

parse_url("ssh://username@domain.tld:password@website.com/path/to"); // works
parse_url("ssh://username@domain.tld:pass@word@website.com/path/to"); // works
parse_url("ssh://user:name:password@website.com/path/to"); // doesn't work

That said, the last example - the only one that doesn't work - shouldn't work, anyway, per RFC7617:

   Furthermore, a user-id containing a colon character is invalid, as
   the first colon in a user-pass string separates user-id and password
   from one another; text after the first colon is part of the password.
   User-ids containing colons cannot be encoded in user-pass strings.

So the behavior seems to be correct?

What is the special character you're having an issue with and is the special character in the username or password?

@michael-lavaveshkul
Copy link
Author

Thanks for taking a look! Don't feel too pressured to look into this, as the workaround works just fine for my use case. Also I haven't dug too deep into the specifications so my understanding may be incorrect.

For the specific time I ran into this, my password has a slash in it. So for this example if I set my password to "pass/word" it would end up being like

parse_url('sftp://user:pass/word@webiste.com/path/to'); // returns false
parse_url('sftp://user:pass%2Fword@website.com/path/to'); // ['password' => 'pass%2Fword',...]

My username contains just letters, numbers, and underscores in the ascii, so there was nothing in my username specifically that would mess with parse_url.

@terrafrost
Copy link
Member

https://www.php.net/ssh2 works just fine with pa/ss but, you're right, parse_url, apparently, doesn't.

@terrafrost
Copy link
Member

Ideally, phpseclib would work in the same way https://www.php.net/ssh2 works. This particular component was designed to be drop-in compatible with https://www.php.net/ssh2's stream functionality. Making it so that everything is urldecode'd wouldn't really be consistent with that as that's not what https://www.php.net/ssh2 does.

Here's what ssh2 does:

	if (h) {
		/* Starting with 5.6.28, 7.0.13 need to be clean, else php_url_parse will fail */
		char *tmp = estrdup(path);

		strncpy(tmp + (h-path), h + sizeof("Resource id #")-1, strlen(tmp)-sizeof("Resource id #"));
		resource = php_url_parse(tmp);
		efree(tmp);
	} else {
		resource = php_url_parse(path);
	}

Source: https://github.com/php/pecl-networking-ssh2/blob/83da7146c7ecf1aa84f24faa1de68cc63c284880/ssh2_fopen_wrappers.c#L249-L258

php_url_parse is basically just a wrapper for php_url_parse_ex2 which, in turn, is called for PHP's implementation of parse_url:

	resource = php_url_parse_ex2(str, str_len, &has_port);
	if (resource == NULL) {
		/* @todo Find a method to determine why php_url_parse_ex() failed */
		RETURN_FALSE;
	}

Source: https://github.com/php/php-src/blob/cd0cd3d31ee5e1b0f2cd8899b754a03578b5caef/ext/standard/url.c#L346-L350

That's also the only place where parse_url returns false. So if ssh2 and parse_url are getting the same data then why is one failing and the other isn't?

I think for the time being imma just let this issue remain open. Might not be a bad idea to add a note about this issue in the documentation...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants