Sign in to follow this  

AS 2.0 delayed - security risk with parameter references

This topic is 4737 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I just discovered another difference between the GNUC and MSVC compilers that will delay the release of AS 2.0 a bit. If a class has a defined destructor, GNUC never passes arguments of this type by value. Instead it makes a local copy of the object and passes a reference to that object to the function. The calling function is also responsible for freeing this object afterwards. MSVC on the other hand passes all kinds of objects directly on the stack. Fortunately I don't think this will be too difficult to fix because of the way AS 2.0 handles objects, but there will likely be a delay of at least one week. Sometimes I curse my decision to allow AS to call normal C/C++ functions/methods. There just don't seem to be any standard in this. This makes it very difficult to port AS to other compilers. For AS 2.x I will add a generic calling convention where the application function will receive a pointer to the context stack directly and have special methods for manipulating the stack. Once that is done, compilers other than MSVC and GNUC will at least be able to use the generic calling convention. [Edited by - WitchLord on December 16, 2004 11:07:33 AM]

Share this post


Link to post
Share on other sites
Alright, I managed fix the problem with GNUC and objects.

Still I recently thought of a huge security risk that I have to fix for AS 2.0 (though not for the first WIP). It's about passing parameters by reference, it's simply not safe enough. Example:


void func(int[] &array, int &value)
{
array.resize(5);
value = 2;
}

void func2()
{
int[] array(10);
func(array, array[9]);
}


Calling func2() will make func() write to memory that is not allocated. In this small example this probably won't cause a problem, but in other situations it could be a huge security risk. Might even be possible for hackers to cause harm to the user's computer.

I have to figure out a way to make parameters by reference safe, or I'll have to remove them completely.

I'm open to suggestions here.

For objects that the library can control the life time for this is not a problem, the library just need to add an extra reference to the object before the function call, and then release it afterwards.

But for primitives or objects that don't allow the control of their lifetime the compiler would have to do something else. Maybe create a local copy of the value, and pass it's reference to the function, and when the function returns copy the value back to it's original location (if it still exists). This would solve the above example in that the script would through an out-of-range exception once the function returned. I think this is the best solution right now though it does have it's own set of problems. The script code would have to compute the argument expression twice, once to get the value that must be passed to the function, and another time to store the value returned in the parameter. This could create inconsistencies that I would prefer avoiding, the expression for an object argument is computed once, but the expression for primitives are computed twice.

Another solution would be to disable parameters by reference completely, but instead allow multiple return values. I kind of like this idea, but it would require a big change to the language that I don't want to do right now. It would still be possible to register normal C/C++ functions.

I haven't found a solution that I really like yet, but I'll continue to analyse this.

Share this post


Link to post
Share on other sites
Quote:
Original post by WitchLord
I'm open to suggestions here.


You'll need to use something like manually implementing fat-pointers, which keep track of the size of the array. This way you can gracefully quit when the program accesses memory outside the bounds of the array. There isn't really any other ways around these types of problems without restricting program behaviour (you can only do certain things with certain types), unless you want to go along the awful C route of undefined behaviour and buffer-overflows.

Share this post


Link to post
Share on other sites
If I were making a programming language for writing applications I could have left this security hole open, just like C/C++ does. But as a scripting library it is out of the question.

AngelScript already do bounds checking for arrays, but it doesn't solve this problem, because the function that received the parameter reference doesn't know where it came from.

I think I will be forced to remove parameter references altogether, and go with my second idea of allowing multiple return values. This will affect the scripting language, but it shouldn't have to affect the library interface too much.

Since AS will support object handles, these can still be used to pass objects by reference, but primitive values cannot be passed by reference.

Share this post


Link to post
Share on other sites
I think the array should be implemented in a different way. Since element 9 still has open references, it shouldn't be possible to "delete" it by shrinking the array. The resize method should probably check the reference count of all elements that are about to be removed.

Share this post


Link to post
Share on other sites
If such things CAN happen, there's a flaw in your reference counting system. AS should simply refuse to remove/delete something that still has open references. This does sound a bit like bashing, but that's the whole purpose of the reference counting.

Share this post


Link to post
Share on other sites
In my opinion the result of the operation would be an error such as unable to delete an element from a primitive array that has a reference count greater than 1 (Assuming that the array itself holds the first reference for each of its elements).

Share this post


Link to post
Share on other sites
I hear you, and I agree, partly. If AngelScript was not so open, allowing almost any type of object to be registered (even overriding the default array object) I could have implemented it like you said, by adding reference counters for each element (or some other sort of locking mecanism). But as it is I think it would not be beneficial to the library.

But I will analyze this more before making the final decision.

Thank you all for your concern for AngelScript, I will do my best not disappoint you.

Share this post


Link to post
Share on other sites
I've been able to fix the security issue with the parameter references so now I'm back on the normal schedule.

What AngelScript now does is that it always passes a reference to a temporary variable instead of the real variable. The script language has also been modified so that parameter references must be marked with either in, out, or inout so that the compiler can avoid making unnecessary copies. For in parameters the argument expression is evaluated before the call. For out parameters the expression is evaluated after the call. For inout parameters the expression is evaluated both before and after the call. In this case the compiler gives a warning if the expression is complex, i.e. if it uses any function calls, incremental operators, etc.

I believe this is the best solution that I can make while allowing parameter references. Obviously there are no performance advantages of passing parameters by reference because of this, in fact it might even have a performance impact. In some cases it is justifiable though, for example when functions need to return more than one value.

With the introduction of object handles, these will be a better way of passing objects, as they will simply increase the reference count of the object instead of making a copy of them.

The way I solved the parameter references will also allow me to support property get/set functions which is something I've been asked for and that I believe is a good thing. With get/set functions it is possible to add automatic notifications when a property is altered or used.

Regards,
Andreas

Share this post


Link to post
Share on other sites

This topic is 4737 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this