libscript: Prevent null dereference on error when property has no getter

peter-b · peter-b · commit 9115ea4e869b · 2016-10-24T18:03:06.000+01:00
The `MCScriptGetTypeOfPropertyInModule()` function could deference a
null pointer when attempting to determine the get type for the
specified property.  This function was only used in the
`MCScriptThrowInvalidValueForPropertyError()` error dispatch helper,
which in turn was only used from contexts where the VM had already
resolved a specific type for the property and had _just_ failed to
compare a value with it.

This patch removes the `MCScriptGetTypeOfPropertyInModule()` function
entirely, and refactors `MCScriptThrowInvalidValueForPropertyError()`
to take the failed property type as an argument.

By doing so it also corrects another bug: when the type required when
setting a property is different from the type obtained by getting the
property, the error message when failing to set the property would be
misleading.

Coverity-Id: 137924
diff --git a/libscript/src/script-error.cpp b/libscript/src/script-error.cpp
@@ -58,6 +58,7 @@ MCScriptThrowPropertyUsedBeforeAssignedError(MCScriptInstanceRef p_instance,
 bool
 MCScriptThrowInvalidValueForPropertyError(MCScriptInstanceRef p_instance,
 										  MCScriptPropertyDefinition *p_property_def,
+                                          MCTypeInfoRef p_property_type,
 										  MCValueRef p_provided_value)
 {
 	return MCErrorCreateAndThrow(kMCScriptInvalidPropertyValueErrorTypeInfo,
@@ -67,8 +68,7 @@ MCScriptThrowInvalidValueForPropertyError(MCScriptInstanceRef p_instance,
 								 MCScriptGetNameOfDefinitionInModule(p_instance->module,
 																	 p_property_def),
 								 "type",
-								 MCScriptGetTypeOfPropertyInModule(p_instance->module,
-																   p_property_def),
+	                             p_property_type,
 								 "value",
 								 p_provided_value,
 								 nil);
diff --git a/libscript/src/script-instance.cpp b/libscript/src/script-instance.cpp
@@ -415,6 +415,7 @@ MCScriptSetPropertyInInstance(MCScriptInstanceRef self,
 	{
 		return MCScriptThrowInvalidValueForPropertyError(self,
 														 t_property_def,
+		                                                 t_property_type,
 														 p_value);
 	}
 	
diff --git a/libscript/src/script-module.cpp b/libscript/src/script-module.cpp
@@ -1096,40 +1096,6 @@ MCScriptGetNameOfDefinitionInModule(MCScriptModuleRef self,
     return kMCEmptyName;
 }
 
-MCTypeInfoRef
-MCScriptGetTypeOfPropertyInModule(MCScriptModuleRef self,
-								  MCScriptPropertyDefinition *p_property_def)
-{
-	MCScriptDefinition *t_getter =
-			p_property_def->getter != 0 ? self->definitions[p_property_def->getter - 1] : nil;
-	
-	switch(t_getter->kind)
-	{
-		case kMCScriptDefinitionKindVariable:
-		{
-			return __MCScriptGetTypeWithIndexInModule(self,
-													  static_cast<MCScriptVariableDefinition*>(t_getter)->type);
-		}
-		break;
-			
-		case kMCScriptDefinitionKindHandler:
-		{
-			return MCScriptGetTypeOfReturnValueInModule(self,
-														static_cast<MCScriptCommonHandlerDefinition*>(t_getter));
-		}
-		break;
-		
-		default:
-		{
-			/* LOAD CHECK */
-			__MCScriptUnreachable__("property getter is not a variable or handler");
-		}
-		break;
-	}
-	
-	return kMCNullTypeInfo;
-}
-
 MCNameRef
 MCScriptGetNameOfParameterInModule(MCScriptModuleRef self,
 								   MCScriptCommonHandlerDefinition *p_handler_def,
diff --git a/libscript/src/script-private.h b/libscript/src/script-private.h
@@ -406,7 +406,6 @@ MCNameRef MCScriptGetNameOfLocalVariableInModule(MCScriptModuleRef module, MCScr
 MCNameRef MCScriptGetNameOfGlobalVariableInModule(MCScriptModuleRef module, MCScriptVariableDefinition *definition);
 MCNameRef MCScriptGetNameOfParameterInModule(MCScriptModuleRef module, MCScriptCommonHandlerDefinition *definition, uindex_t index);
 
-MCTypeInfoRef MCScriptGetTypeOfPropertyInModule(MCScriptModuleRef module, MCScriptPropertyDefinition *definition);
 MCTypeInfoRef MCScriptGetTypeOfLocalVariableInModule(MCScriptModuleRef module, MCScriptHandlerDefinition *definition, uindex_t index);
 MCTypeInfoRef MCScriptGetTypeOfGlobalVariableInModule(MCScriptModuleRef module, MCScriptVariableDefinition *definition);
 MCTypeInfoRef MCScriptGetTypeOfParameterInModule(MCScriptModuleRef module, MCScriptCommonHandlerDefinition *definition, uindex_t index);
@@ -486,6 +485,7 @@ MCScriptThrowPropertyUsedBeforeAssignedError(MCScriptInstanceRef instance,
 bool
 MCScriptThrowInvalidValueForPropertyError(MCScriptInstanceRef instance,
 										  MCScriptPropertyDefinition *property_def,
+                                          MCTypeInfoRef property_type,
 										  MCValueRef provided_value);
 
 bool

Original file line number	Diff line number	Diff line change
`@@ -415,6 +415,7 @@ MCScriptSetPropertyInInstance(MCScriptInstanceRef self,`
`415`	`415`	`{`
`416`	`416`	`return MCScriptThrowInvalidValueForPropertyError(self,`
`417`	`417`	`t_property_def,`
	`418`	`+ t_property_type,`
`418`	`419`	`p_value);`
`419`	`420`	`}`
`420`	`421`