E. Choroba

For October’s turn of the CPAN Pull Request Challenge, I was assigned Net::SFTP::Foreign. I concentrated on one of the latest issues reported: in recent Perl versions, the module didn’t work in taint mode.

The failure was rather easy to reproduce:

$remote->setcwd('/');

The error message was the first clue, as it mentioned stat:

Insecure argument '/' on 'stat' method call while running with -T switch at ...

I analyzed the source of the setcwd method and found the offending lines:

my $a = $sftp->stat($cwd)
    or return undef;

Clearly, $cwd was tainted here. Where did it come from? Only few lines above, it was populated as

$cwd = $sftp->realpath($cwd);

So, realpath returned a tainted string. Its definition was a bit harder to understand:

*realpath = _gen_getpath_method(SSH2_FXP_REALPATH,
                                SFTP_ERR_REMOTE_REALPATH_FAILED,
                                "realpath");

_gen_getpath_method generates a subroutine that returns the path as follows:

return $sftp->_fs_decode($msg->get_str);

So, in one of these places, the string should be untainted—but I wasn’t sure which one. Therefore, I only added my investigation as a comment to the bug report. The comment seemed to help SALVA, as he fixed the issue two days later (the path was untainted in the second spot, after the call to realpath).

Win-lose situation

CPAN was shaved a bug, but my comment to the Request Tracker wasn’t a pull request. I tried to find some other issues or testers’ problems, but the module seemed well maintained, offering no more opportunities.

In the end, I noticed the last test was skipped with

Test::Spelling required for testing POD spelling

I installed Test::Spelling, reran the tests and voilà: there were several unknown words reported:

ls Wikipedia doc docs stat stats dos ssh

None of them seemed like a typo, so I added them to the exception list in my pull request. Now, I just needed three more pull requests to get my T-shirt in Hacktoberfest.