Add additional reliability and stability notes to modules

[adfoster-r7]

Add additional reliability and stability notes to modules

Verification

• Verify the notes are cromulent

[gwillcox-r7]

I've reviewed a few of these 6 files but already I'm seeing a worrying trend to use the same base template of CRASH_SAFE and REPEATABLE_SESSION for all of these files regardless of the nature of the vulnerability involved. I've noted a few cases here but ideally I think this point needs further consideration and will be pausing my review until we can come to some conclusion on this, as I'd prefer not to be giving people the false impression that these exploits are more reliable than they actually are.

[gwillcox-r7]

Is this a good option given it this involves a race condition and two information leaks? Seems to me there is a small chance that the race condition might be lost here.

[adfoster-r7]

The exploit runs in a loop; so it should be repeatable

metasploit-framework/modules/exploits/windows/scada/rockwell_factorytalk_rce.rb

Lines 205 to 212 in 4cab6cd

	racer = Thread.new do
	loop do
	res = send_to_factory("/#{@shelly}")
	if res.code == 200
	print_good("#{peer} - We've won the race condition, shell incoming!")
	break
	end
	end

[gwillcox-r7]

That's just a loop that the race condition code is trying to execute where it repeatedly tries to access the file we upload until it gets a 200 OK response. Whilst that certainly increases the chances, it doesn't guarantee that it will win the race every time. So long is there is a chance the race might be lost we shouldn't be using REPEATABLE_SESSION here. I think something inbetween UNRELIABLE_SESSION and FIRST_ATTEMPT_FAIL would be great but unfortunately it doesn't look like we have a definition for "might fail more than the first time, but overall not terrible on reliability".

[gwillcox-r7]

@adfoster-r7 Do you have any updates on the above? Would love to get this landed but have some outstanding concerns.

[adfoster-r7]

Hoping to get this sorted next week-ish 👍

[gwillcox-r7]

Bit conflicted on the CONFIG_CHANGES since technically we aren't modifying a config file here, but we are adding a new bot. I'm not sure if we should remove CONFIG_CHANGES here on that basis.

[gwillcox-r7]

Bump, any update on this @adfoster-r7? This looks close to being able to be landed, just needs the issues mentioned above addressed.

[gwillcox-r7]

Sending this to the attic due to a lack of updates. We'll get back to this if its still a priority.

[github-actions]

Thanks for your contribution to Metasploit Framework! We've looked at this pull request, and we agree that it seems like a good addition to Metasploit, but it looks like it is not quite ready to land. We've labeled it attic and closed it for now.

What does this generally mean? It could be one or more of several things:

• It doesn't look like there has been any activity on this pull request in a while
• We may not have the proper access or equipment to test this pull request, or the contributor doesn't have time to work on it right now.
• Sometimes the implementation isn't quite right and a different approach is necessary.

We would love to land this pull request when it's ready. If you have a chance to address all comments, we would be happy to reopen and discuss how to merge this!

[sjanusz-r7]

Release Notes

Reliability and stability notes that have been previously missing have been added to some modules.