Error-code spoofing

[graydon]

When a contract returns an error the ABI doesn't differentiate the guest-level values Ok(Error) from Err(Error) from Error. This is fine and expected, but with_frame in the host attempts to "fix" this by funneling Ok(Error) into Err(HostError) back in host-land, and this makes it possible for user code to "spoof" error codes -- return things other than ScErrorType::ContractError.

For most error codes this is at best a minor source of confusion, but we have some paths that care about the difference, eg. try_call decides whether an error is recoverable or unrecoverable based on the error type, and also we just generally want ScErrorCode::InternalError to only represent "there's a problem in the host's own logic" unambiguously.

Naturally the fuzzer discovered this game immediately: it can generate InternalError by writing a contract consisting of the constant value InternalError.

The fix for this is probably to get with_frame out of the business of massaging Ok(Error) returns and have call, try_call and whatever else emulates calling and error-propagation in the native-contract case look at the Ok(Error) cases they specifically want to admit -- Ok(ScErrorType::ContractError) alone -- and map that to Err(...), and the rest into different InvalidAction codes or something.

rs-soroban-env/soroban-env-common/src/arbitrary.rs

Lines 14 to 21 in 6f69811

	// FIXME: fuzzer discovered that it can just return "InternalError" from
	// a contract to make the host think it had an InternalError. See
	// https://github.com/stellar/rs-soroban-env/issues/1175
	if error.is_code(ScErrorCode::InternalError) {
	Err(arbitrary::Error::IncorrectFormat)
	} else {
	Ok(error)
	}

[leighmcculloch]

This issue has a knock on affect on the SDK that required this change, but it is unclear if that change is the right change:

• Update soroban-env-* deps rs-soroban-sdk#1140

And requires at least this follow up:

• Fix and reenable prop test arbitrary tests rs-soroban-sdk#1141

[graydon]

Done in #1209