For October’s turn of the CPAN Pull Request Challenge, I was assigned Net::SFTP::Foreign. Iconcentrated 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 setcwdmethod and found the offending lines:my $a = $sftp->stat($cwd) or return undef;
Clearly, $cwdwas tainted here. Where did it come from? Only few lines above, it was populated as$cwd = $sftp->realpath($cwd);
So, realpathreturned 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_methodgenerates 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, Ionly 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. Itried 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 withTest::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.