fongoses / amarino

Automatically exported from code.google.com/p/amarino
Other
0 stars 0 forks source link

Error function pointer variable can point to arbitrary memory #11

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Initialize MeetAndroid with default constructor
2. Send data to arduino with unregistered flag

What is the expected output? What do you see instead?

Undefined. Since the there is not a default error function (nor is it 
initialized to zero and checked). The errorFunc is pointing to arbitrary memory 
which will cause undefined behavior depending on what it is pointing to.

What version of Amarino are you using?

2

Please provide any additional information below.

You can resolve this issue by doing any of the following:

* Provide a default error function
* Initialize the errorFunc to zero in default constructor and check it before 
invoking function (processCommand)

Original issue reported on code.google.com by dewi...@gmail.com on 25 Oct 2010 at 11:30

GoogleCodeExporter commented 9 years ago
It is already checked if an error function is defined. If you use the default 
constructor, the error function will never be called. You think that is not 
enough?

Look at line 34 of MeetAndroid.cpp

if (customErrorFunc)
  errorFunc(buffer[0], getArrayLength());
else {
  Serial.print("No func. attached for: ");
  Serial.print(buffer[0]);
}

Original comment by bonifaz....@gmail.com on 26 Oct 2010 at 9:32

GoogleCodeExporter commented 9 years ago
I don't think so because that check is only done if flag is out the allowable 
bounds of the intFunc subscript. If the flag is in bounds (which is usually the 
case), then the function pointer is extracted from the array and executed. By 
default, the intFunc array is initialized with the 'errorFunc' function pointer 
variable. If the 'errorFunc' is not set then it is pointing to arbitrary 
memory; which is the case when the default constructor is used.

For example,

1. Initialize with default constructor
2. Register for flag 'b'
3. Execute receive in loop()
4. Incoming Bytes -> ['f', 12, 59]
5. Delegate to Process Command
6. Subscript Bounds Check -> OK ( (102 - 48) < 75)
7. Extract Function Pointer -> intFunc[102 - 48]
8. Execute Function -> errorFunc is unset which would cause undefined behavior

Original comment by dewi...@gmail.com on 26 Oct 2010 at 9:53

GoogleCodeExporter commented 9 years ago
Oh yes you are right.

Would you correct the library and attach the new corrected files to this issue. 
I will then commit your changes.

Original comment by bonifaz....@gmail.com on 26 Oct 2010 at 10:24

GoogleCodeExporter commented 9 years ago
I would really appreciate it.

Original comment by bonifaz....@gmail.com on 26 Oct 2010 at 10:25

GoogleCodeExporter commented 9 years ago
ok I did it already...fixed another little bug as well...will commit the code 
in a few minutes.

Original comment by bonifaz....@gmail.com on 26 Oct 2010 at 10:37

GoogleCodeExporter commented 9 years ago

Original comment by bonifaz....@gmail.com on 26 Oct 2010 at 10:38

GoogleCodeExporter commented 9 years ago
Thanks!

Original comment by dewi...@gmail.com on 26 Oct 2010 at 10:41