Create rubocop rule for preferring File.binread over IO.read

[bcoles]

As per:

• Replace IO read on binary files with File binread #16325 (comment)
• Review instances of IO.read and replace with File.binread #16285 (comment)

May also need to check for File.read as per:

• Replace IO read on binary files with File binread #16325 (comment).
• Replace IO read on binary files with File binread #16325 (comment)

[adfoster-r7]

For context, we didn't originally add automation for this to the original PR as there are scenarios where IO.read and File.read work just fine, i.e. for wordlists etc, but not for executable binary files.

There are other scenarios where the user's intentions are ambiguous, i.e. if a user uses an ftp upload module - should the bytes be copied verbatim, or should new lines be normalized between the client/target hosts. Other tools solve this by letting the user specify ascii/binary modes, i.e. in the case of bsd's ftp client, or just always uploading in binary mode, i.e. samba's smbclient, but it feels like we'd have to introduce a convention to our modules for that.

It'd be cool to automate this with Rubocop, but I'm on the fence about it solving the underlying issue with above ambiguities. I think the nuances of specifying mode rb or r, or using binread vs readlines vs readlines(chomp: true) may be lost to drive-by contributors, who will just copy/paste from existing modules until rubocop is happy. Although a rule would at least bring an awareness to the issue either way, which is definitely better.

[adfoster-r7]

The above aside; msftidy does some checks for specifying binary mode with File.open over here:

metasploit-framework/tools/dev/msftidy.rb

Lines 680 to 685 in 45dcfda

	url_ok = false if ln =~ /\.com\/projects\/Framework/
	if ln =~ /File\.open/ and ln =~ /[\"\'][arw]/
	if not ln =~ /[\"\'][wra]\+?b\+?[\"\']/
	warn("File.open without binary mode", idx)
	end
	end

A dedicated rubocop rule will definitely be smarter than this regex 👍

[github-actions]

Hi!

This issue has been left open with no activity for a while now.

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 30 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request.