Memory unauthorized access issues in byte2search in Cyrus IMAP

[Bron Gondwana]

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20161107/4743f3b6/attachment.html>
-------------- 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/20161107/4743f3b6/attachment.png>