Memory unauthorized access issues in byte2search in Cyrus IMAP
Egoitz Aurrekoetxea
egoitz at sarenet.es
Tue Nov 8 05:00:47 EST 2016
Hi Bron!,
Yes true! I thought at first sight you were trying to make it compatible
with any value int or size_t could have platform independent but
obviously is a totally non sense... because we need the
allocated space for the pointer to be able to fit all the data we need
to enter in it.... you were just saying: "allocate n times (the number
of times the first char exists in pattern) the size of the first
element unit stored in the pointer (and for this calc accessed as an
array) which is defined as a size_t type"...
Ttrue Bron :)
Thanks a lot mate ;)
Cheers!
El 7/11/16 a las 21:19, Bron Gondwana via Cyrus-devel escribió:
> Either is fine. The nice thing about this way is that if the
> underlying definition of s->starts is changed, it only needs to be
> changed in one location.
>
> Bron.
>
>
> On Tue, 8 Nov 2016, at 02:26, Egoitz Aurrekoetxea via Cyrus-devel wrote:
>>
>> Then you will finally leave the line as shown the next line? :
>>
>>
>> s->starts = xmalloc(s->max_start * sizeof(s->starts[0]));
>>
>>
>> I mean instead of sizeof(size_t) ? perhaps more compatible as Ellie
>> committed it and without having issues on platforms which define
>> differently size_t?.
>>
>>
>> It's for patching the same way you are going to leave it in your
>> repository and the way the new release would come....
>>
>>
>> Best regards,
>>
>>
>>
>> El 7/11/16 a las 14:07, Bron Gondwana via Cyrus-devel escribió:
>>> cb67ecd9 (ellie timoney 2016-11-02 10:31:07 +1100 726)
>>> s->starts = xmalloc(s->max_start * sizeof(s->starts[0]));
>>>
>>> I see it was already fixed last week.
>>>
>>> Bron.
>>>
>>>
>>> On Mon, 7 Nov 2016, at 23:23, Egoitz Aurrekoetxea via Cyrus-devel wrote:
>>>>
>>>> God morning!!
>>>>
>>>>
>>>> Thanks a lot for the confirmation!!
>>>>
>>>>
>>>> Best regards,
>>>>
>>>>
>>>> El 7/11/16 a las 12:43, Bron Gondwana via Cyrus-devel escribió:
>>>>> You're absolutely right, it should be changed. If you have a
>>>>> platform where sizeof(int) != sizeof(size_t) then you'll have
>>>>> problems with that.
>>>>>
>>>>> I'll fix it on the 2.3 branch, though we probably won't cut a
>>>>> release from it immediately. It's not supported any more. We
>>>>> released 2.4.0 over 6 years ago now!
>>>>>
>>>>> Bron.
>>>>>
>>>>> On Mon, 7 Nov 2016, at 20:43, Egoitz Aurrekoetxea via Cyrus-devel
>>>>> wrote:
>>>>>> Good morning,
>>>>>>
>>>>>>
>>>>>> I have been checking the Cyrus IMAP 2.3.19 and 2.3.18 code
>>>>>> because I have observed some issues in UID SORT commands in the
>>>>>> IMAP protocol. When performing a command
>>>>>>
>>>>>> like ". UID SORT (SIZE) US-ASCII ALL TEXT avanzada" in a mailbox
>>>>>> where matches were found caused you to obtain in a debug (or non
>>>>>> debug I think) log the following entry :
>>>>>>
>>>>>> Oct 31 09:17:21 hostname master[78064]: process 78268 exited,
>>>>>> signaled to death by 11
>>>>>>
>>>>>> Lines like this are seen when a process has been signaled by the
>>>>>> kernel with signal 11. Have been reading this signal is sent to a
>>>>>> proccess when it performs an unauthorized memory
>>>>>>
>>>>>> access attemp (an out of the own variable, pointer... etc,
>>>>>> storage room). After debugging the code with GDB and doing
>>>>>> several checks, have seen the issue came from the byte2search()
>>>>>>
>>>>>> function when a piece of the string s->substr was trying to be
>>>>>> stored in b. Concretely the third if in the loop :
>>>>>>
>>>>>>
>>>>>> for (i = 0, cur = 0; i < s->max_start; i++) {
>>>>>> /* no more active offsets */
>>>>>> if (s->starts[i] == -1)
>>>>>> break;
>>>>>>
>>>>>> /* if we've passed one that's not ongoing, copy back */
>>>>>> if (cur < i) {
>>>>>> s->starts[cur] = s->starts[i];
>>>>>> }
>>>>>> /* check that the substring is still maching */
>>>>>> if (b == s->substr[s->offset - s->starts[i]]) {
>>>>>>
>>>>>>
>>>>>> The issue was caused there because s->starts[i] in this place,
>>>>>> was not being able to be accesed because it was pointing to to
>>>>>> data outside s->starts. After searching where this array was
>>>>>> being initialized
>>>>>>
>>>>>> and it's memory allocated (which was in search_init function), I
>>>>>> tried to allocate 10 bytes more for that pointer. After doing it,
>>>>>> there were no more issues. So I tried allocating just one byte
>>>>>> more which it seemed
>>>>>>
>>>>>> to be enough too (at least for the patterns I have searched for).
>>>>>> At this moment I understood this pointer (s->starts which was a
>>>>>> search_state->substr pointer inside the search_state structure)
>>>>>> was not having
>>>>>>
>>>>>> enough room for all the content needed to be stored, or at least
>>>>>> accesed when calling it. I checked then the code of Cyrus 2.3.18
>>>>>> and 2.3.19 but didn't see any kind of differences in the part of
>>>>>> the memory
>>>>>>
>>>>>> allocation (in search_init()) or usage (in bytesearch) for
>>>>>> s->starts. I deciced to check Cyrus 2.4 code and I saw it's room
>>>>>> was being allocated the following way :
>>>>>>
>>>>>>
>>>>>> s->starts = xmalloc(s->max_start * sizeof(size_t));
>>>>>>
>>>>>>
>>>>>> instead of that in 2.3 was done :
>>>>>>
>>>>>>
>>>>>> s->starts = xmalloc(s->max_start * sizeof(int));
>>>>>>
>>>>>>
>>>>>> So I understood s->starts should be allocated to the size of a
>>>>>> size_t type defined variable size, instead to the size of an
>>>>>> integer variable n times. After replacing it, has seen
>>>>>> definitively all seemed to be
>>>>>>
>>>>>> working. So wouldn't Cyrus 2.3 sources have this allocation in
>>>>>> search_init done with sizeof(size_t) instead of the sizeof(int)?.
>>>>>> I think this is important because else, when the first character of a
>>>>>>
>>>>>> pattern is repeated more than one time, the pattern has a would
>>>>>> say patlen of 8-9 bytes and matches exist in the mailbox, that
>>>>>> search would end up with a proccess died due to a signal 11.
>>>>>>
>>>>>>
>>>>>> My env is FreeBSD RELENG_9_0 OS with a Cyrus 2.3.18_1 port. Am I
>>>>>> wrong, shouldn't that allocation be changed?.
>>>>>>
>>>>>>
>>>>>> Thanks a lot for your time,
>>>>>>
>>>>>> Best regards,
>>>>>>
>>>>>> --
>>>>>>
>>>>>>
>>>>>>
>>>>>> sarenet
>>>>>> *Egoitz Aurrekoetxea*
>>>>>> Departamento de sistemas
>>>>>> 944 209 470
>>>>>> Parque Tecnológico. Edificio 103
>>>>>> 48170 Zamudio (Bizkaia)
>>>>>> egoitz at sarenet.es <mailto:egoitz at sarenet.es>
>>>>>> www.sarenet.es <http://www.sarenet.es>
>>>>>>
>>>>>> Antes de imprimir este correo electrónico piense si es necesario
>>>>>> hacerlo.
>>>>>>
>>>>>
>>>>> --
>>>>> Bron Gondwana
>>>>> brong at fastmail.fm <mailto:brong at fastmail.fm>
>>>>>
>>>>>
>>>>
>>>> --
>>>>
>>>>
>>>>
>>>> sarenet
>>>> *Egoitz Aurrekoetxea*
>>>> Departamento de sistemas
>>>> 944 209 470
>>>> Parque Tecnológico. Edificio 103
>>>> 48170 Zamudio (Bizkaia)
>>>> egoitz at sarenet.es <mailto:egoitz at sarenet.es>
>>>> www.sarenet.es <http://www.sarenet.es>
>>>>
>>>> Antes de imprimir este correo electrónico piense si es necesario
>>>> hacerlo.
>>>>
>>>>
>>>
>>> --
>>> Bron Gondwana
>>> brong at fastmail.fm <mailto:brong at fastmail.fm>
>>>
>>>
>>
>> --
>>
>>
>>
>> sarenet
>> *Egoitz Aurrekoetxea*
>> Departamento de sistemas
>> 944 209 470
>> Parque Tecnológico. Edificio 103
>> 48170 Zamudio (Bizkaia)
>> egoitz at sarenet.es
>> www.sarenet.es <http://www.sarenet.es>
>>
>> Antes de imprimir este correo electrónico piense si es necesario hacerlo.
>>
>>
>>
>
> --
> Bron Gondwana
> brong at fastmail.fm
>
>
--
sarenet
*Egoitz Aurrekoetxea*
Departamento de sistemas
944 209 470
Parque Tecnológico. Edificio 103
48170 Zamudio (Bizkaia)
egoitz at sarenet.es <mailto:egoitz at sarenet.es>
www.sarenet.es <http://www.sarenet.es>
Antes de imprimir este correo electrónico piense si es necesario hacerlo.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20161108/29fb3abc/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 1545 bytes
Desc: not available
URL: <http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20161108/29fb3abc/attachment-0004.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 1545 bytes
Desc: not available
URL: <http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20161108/29fb3abc/attachment-0005.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 1545 bytes
Desc: not available
URL: <http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20161108/29fb3abc/attachment-0006.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: LogoSarenetEmails.png
Type: image/png
Size: 1545 bytes
Desc: not available
URL: <http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20161108/29fb3abc/attachment-0007.png>
More information about the Cyrus-devel
mailing list