Differential D3267 Diff 9266 src/script/interpreter.cpp

Changeset View

Standalone View

src/script/interpreter.cpp

Show First 20 Lines • Show All 1,117 Lines • ▼ Show 20 Lines	try {
if (stack.size() < 2) {		if (stack.size() < 2) {
return set_error(		return set_error(
serror, SCRIPT_ERR_INVALID_STACK_OPERATION);		serror, SCRIPT_ERR_INVALID_STACK_OPERATION);
}		}

const valtype &data = stacktop(-2);		const valtype &data = stacktop(-2);

// Make sure the split point is appropriate.		// Make sure the split point is appropriate.
uint64_t position =		int position =
CScriptNum(stacktop(-1), fRequireMinimal).getint();		CScriptNum(stacktop(-1), fRequireMinimal).getint();
if (position > data.size()) {		if (position < 0 \|\| position > (int)data.size()) {
		markblundebergAuthorUnsubmitted Done Inline Actions note -- the (int) cast style is in line with other opcodes like OP_ROLL, OP_CHECKMULTISIG markblundeberg: note -- the (int) cast style is in line with other opcodes like OP_ROLL, OP_CHECKMULTISIG
		deadalnixUnsubmitted Not Done Inline Actions You added the assumption that data.size() fits into an int. Which it does, but regardless, baking more assumptions into the code is a very clear regression. deadalnix: You added the assumption that data.size() fits into an int. Which it does, but regardless…
return set_error(serror,		return set_error(serror,
SCRIPT_ERR_INVALID_SPLIT_RANGE);		SCRIPT_ERR_INVALID_SPLIT_RANGE);
}		}

// Prepare the results in their own buffer as `data`		// Prepare the results in their own buffer as `data`
// will be invalidated.		// will be invalidated.
valtype n1(data.begin(), data.begin() + position);		valtype n1(data.begin(), data.begin() + position);
valtype n2(data.begin() + position, data.end());		valtype n2(data.begin() + position, data.end());

// Replace existing stack values by the new values.		// Replace existing stack values by the new values.
stacktop(-2) = std::move(n1);		stacktop(-2) = std::move(n1);
stacktop(-1) = std::move(n2);		stacktop(-1) = std::move(n2);
} break;		} break;

//		//
// Conversion operations		// Conversion operations
//		//
case OP_NUM2BIN: {		case OP_NUM2BIN: {
// (in size -- out)		// (in size -- out)
if (stack.size() < 2) {		if (stack.size() < 2) {
return set_error(		return set_error(
serror, SCRIPT_ERR_INVALID_STACK_OPERATION);		serror, SCRIPT_ERR_INVALID_STACK_OPERATION);
}		}

uint64_t size =		CScriptNum bnSize(stacktop(-1), fRequireMinimal);
		deadalnixUnsubmitted Not Done Inline Actions Why don't you get the size as an int and then test that ? You can still have both checks and a nicer error code this way. deadalnix: Why don't you get the size as an int and then test that ? You can still have both checks and a…
CScriptNum(stacktop(-1), fRequireMinimal).getint();		if (bnSize < 0) {
		return set_error(serror,
		SCRIPT_ERR_NEGATIVE_NUMBER);
		}
		unsigned int size = bnSize.getint();
if (size > MAX_SCRIPT_ELEMENT_SIZE) {		if (size > MAX_SCRIPT_ELEMENT_SIZE) {
return set_error(serror, SCRIPT_ERR_PUSH_SIZE);		return set_error(serror, SCRIPT_ERR_PUSH_SIZE);
		deadalnixUnsubmitted Not Done Inline Actions Even though it is unlikely to matter in practice, it is a good idea to have one check for error condition then detail in there. if (uint64_t(size) > MAX_SCRIPT_ELEMENT_SIZE) { return set_error(serror, size < 0 ? SCRIPT_ERR_NEGATIVE_NUMBER : SCRIPT_ERR_PUSH_SIZE); } deadalnix: Even though it is unlikely to matter in practice, it is a good idea to have one check for error…
}		}

popstack(stack);		popstack(stack);
valtype &rawnum = stacktop(-1);		valtype &rawnum = stacktop(-1);

// Try to see if we can fit that number in the number of		// Try to see if we can fit that number in the number of
// byte requested.		// byte requested.
CScriptNum::MinimallyEncode(rawnum);		CScriptNum::MinimallyEncode(rawnum);
▲ Show 20 Lines • Show All 509 Lines • Show Last 20 Lines