Topics

OSX - Need Ziga and Edouard - Possible bug found (memory leakage)


Fabio IZ0IBA 2220156
 

Hi Ziga and Edouard, i refer to my other post number #745 where i report a segmentation fault after an Apple security update on iMovie.
Since SDRangel won't start as normal user but only with sudo, i spent all day trying to understand what was the issue. At first i investigated all kind of file permission of compiled files and libraries 
but i found that the crash was still present even if i setuid all stuff with special s permission. So was something related to core framework.
A more strict security control has been settled up by Apple to check memory areas accessed by the executable.  So, i guessed there is probably a memory leakage somewhere.  So i debugged step by step the program in QT creator and found this :

The program crash on start (as normal user) on "glscopegui.cpp" on line 383   
ScopeVis::TriggerData triggerData = m_scopeVis->getTriggerData(iTrigger);
when iTrigger is greater than zero. 
The For cycle starts from iTrigger=0 to nbTriggerSaved  and this value is read as 4 by the preceding instruction
d.readU32(200, &nbTriggersSaved, 1);

The getTriggerData() method is defined in "scopevis.h" in this way 
const TriggerData& getTriggerData(uint32_t triggerIndex) const { return m_triggerConditions[triggerIndex]->m_triggerData; }

where we see that there is an attempt to read an array m_triggerConditions of structs at index "triggerIndex"
Since the m_triggerConditions is defined this way in scopevis.h 

std::vector<TriggerCondition*> m_triggerConditions

this is a dynamic vector of pointers to structure TriggerCondition but at the beginning it seems (to me) be composed only of one element.
So when the "for cycle" attempts to read m_scopeVis->getTriggerData(iTrigger) with iTrigger >0 say 1 it results in accessing an array whose memory address is unknown.
This condition is intercepted by the OSX framework and a Segment Fault condition is thrown.

So to resolve this unwanted condition i "as a test" imposed that the nbTriggerSaved was = 1 adding the line 

        nbTriggersSaved=1;

just before the for cycle at line 380 of glscopegui.cpp.
Recompiling and re-deploying the application results in normal start and behaviour. The Channel Analyzer gui is normal and i can also increase the number of trigger saved.

I ask Ziga and Edouard to check the code there to see if my finding is appropriate. It works for me but i don't know if this is a general mod to be included in the main branch of the code.
I also have an explanation why with sudo the application was starting. This because when accessing the memory areas out of boundary the core framework does not complain since access was granted 
to the whole address space.  So this leakage was already present in my opinion, but has been exploited by the Apple security fix i installed a couple of day ago. 

73, Fabio IZ0IBA










Edouard Griffiths
 
Edited

Hi Fabio,

thanks for the inverstigation and report. This is ~2 year old code and I am now not so familiar with it. Having a second look it seems to me it is problematic in many ways. The way the triggers are reconstructed from the saved data seems illogical. At least this is fairly easy to reproduce:

1. On the first default device tab select whatever for the source
2. Add a Channel Analyzer
3. Set 4 triggers (maybe 2 would be enough)
4. Save as a preset
5. Restart SDRangel
6. Select and load the preset => crash

I'm having a closer look at how this deserialization works.

Brgds, Edouard.

Note: issue opened: https://github.com/f4exb/sdrangel/issues/314