Memory unauthorized access issues in byte2search in Cyrus IMAP
Bron Gondwana
brong at fastmail.fm
Mon Nov 7 15:19:46 EST 2016
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
>>>>> 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
>>> 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
> www.sarenet.es
>
> Antes de imprimir este correo electrónico piense si es necesario
> hacerlo.
>
>
>
--
Bron Gondwana
brong at fastmail.fm
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20161108/efa87e98/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Attachment2.2
Type: image/png
Size: 1545 bytes
Desc: not available
URL: <http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20161108/efa87e98/attachment-0003.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Attachment2.3
Type: image/png
Size: 1545 bytes
Desc: not available
URL: <http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20161108/efa87e98/attachment-0004.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/efa87e98/attachment-0005.png>
More information about the Cyrus-devel
mailing list