FreeSWITCH
  1. FreeSWITCH
  2. FS-5566

RFE: Support ${N} syntax for regexp groups expansion

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: freeswitch-core
    • Security Level: public
    • Labels:
      None
    • CPU Architecture:
      x86
    • Kernel:
      Linux
    • Userland:
      GNU/Linux
    • lsb_release:
      Hide
      LSB Version: :base-4.0-amd64:base-4.0-noarch:core-4.0-amd64:core-4.0-noarch:graphics-4.0-amd64:graphics-4.0-noarch:printing-4.0-amd64:printing-4.0-noarch
      Distributor ID: CentOS
      Description: CentOS release 6.4 (Final)
      Release: 6.4
      Codename: Final
      Show
      LSB Version: :base-4.0-amd64:base-4.0-noarch:core-4.0-amd64:core-4.0-noarch:graphics-4.0-amd64:graphics-4.0-noarch:printing-4.0-amd64:printing-4.0-noarch Distributor ID: CentOS Description: CentOS release 6.4 (Final) Release: 6.4 Codename: Final
    • Compiler:
      gcc
    • FreeSWITCH GIT Revision:
      58e3680
    • GIT Master Revision hash::
      yes

      Description

      the actual expasion for matched groups in regexp follows the rule $1, $2 and so on for each group.

      This does not make possibile to concat a number after the group, for example:
      $112345 , where $1 is my first matched group.

      Obviously this can be ambiguos, since you may want group 1 or 112345 (even if groups are limited
      to 256), so the same or similar syntax used by bash or perl can be used:
      ${1}12345 => will do the trick

      some dialplan example:
      http://pastebin.freeswitch.org/21136

        Activity

        Hide
        Anthony Minessale II added a comment -
        http://seclists.org/oss-sec/2013/q3/10

        Was it necessary to paint a negative image of our software over an obscure thing that nobody does and only became a problem because of a very specific configuration inspired by an imaginative user? It reads like, "I tried using this software and I tripped over endless buffer overflows...."

        I don't like when people toot their own horn for finding a security problem especially when it comes with a recipe to reproduce that only an internal user can subject himself to with an atypical config.

        I'm glad that you found this and we are happy to include the patch but its probably not the last bug you will find in this or any other code you have read access to.

        I don't object that you help make people aware of this problem but the wording could have been much better =/


         
        Show
        Anthony Minessale II added a comment - http://seclists.org/oss-sec/2013/q3/10 Was it necessary to paint a negative image of our software over an obscure thing that nobody does and only became a problem because of a very specific configuration inspired by an imaginative user? It reads like, "I tried using this software and I tripped over endless buffer overflows...." I don't like when people toot their own horn for finding a security problem especially when it comes with a recipe to reproduce that only an internal user can subject himself to with an atypical config. I'm glad that you found this and we are happy to include the patch but its probably not the last bug you will find in this or any other code you have read access to. I don't object that you help make people aware of this problem but the wording could have been much better =/  
        Hide
        Michael Tokarev added a comment -
        I don't think I follow. I'm sorry about that.

        First of all, speaking of security, it is almost always some obscure thing that nobody does. Think about a webserver with a fixed 1Mb buffer for an input URL -- it is an obscure thing to request such a long URL for sure, and no user does that. But it is exactly what makes it a wide-open security hole, because it is trivial to request such an URL and trigger the buffer overflow, an exploit will be the same be it 1Mb or 10Mb or any other fixed amount (I'm talking about a situation when the length of input is not checked against this buffer).

        The CVE# request was not about overflowing index[10], which is administrator-controlled (you actually have to write a regex that triggers it, and it will almost always cause freeswitch to crash). It was about the other overflow, the buffer for the replacement string, which is _externally_ controllable. Basically, any configuration that uses unconstrained by length regex grouping (\d+ instead of \d{1,10}) has a high potential of being vulnerable - again, from outside! - due to this. And while this is indeed an "obscure thing that nobody does", it is enough to exploit the server and take control over it. And in this context it stops being "obscure thing" and becomes a plain security hole, and this is exactly what I'm talking about.

        Speaking of the wording - can you imagine _my_ situation? It is the _first_ time I _ever_ looked at the software. And in the _first_ function I saw I found 3 buffer overflows. Maybe I'm just this "lucky" and there's no other bugs in there (I know that bugs are always there, but let's pretend I don't know that for clarity), so I found all remaining 3 bugs in this nice software. But it is very hard to believe that. And my reaction in this context is very much predictable, it is as difficult to think differently: "it is a buggy piece of crap, large piece". Again, please don't get me wrong here, this is just a natural reaction I'm trying to get rid of, but unfortunately it _is_ the first natural reaction that comes. If the authors lets themselves do several of such stupid bugs in a simple function, this, in my dictionary, means that they either don't think or don't know. Again, maybe this only function were written while being drunk, and all the rest are fine, but it is also difficult to believe. The question that pops up is -- what other bugs are there? Is it _all_ like this?

        Do you see what I mean? Sure bugs will be everywhere. But there are different bugs, and there are different _amounts_ of bugs in one simple place. And when one see a simple piece of code which contains 3 stupid bugs at once, one really questions sanity of the authors, this is the natural reaction which is very difficult to get rid of.

        I _am_ trying to get rid of it, really, as my time permits me. That's exactly the reason why I've tried to fix the issues, instead of just moving on. And yes, I was in rather a bad mood after all this, and wrote the CVE# request in that mood too.

        But still, speaking of the wording - looking at it again, I don't see where the wording coult have been much better. Yes it is possible to omit the introductional part where I describe it is my first attempt at its usage -- that's 2 first paragraphs excluding the "Hello" line, -- it is true that it is my first time with freeswitch, but it is not relevant. All the rest are pure technical details describing the issues, with rather regular technical wording. I don't see where it can be improved to be much better.

        Thank you!
        Show
        Michael Tokarev added a comment - I don't think I follow. I'm sorry about that. First of all, speaking of security, it is almost always some obscure thing that nobody does. Think about a webserver with a fixed 1Mb buffer for an input URL -- it is an obscure thing to request such a long URL for sure, and no user does that. But it is exactly what makes it a wide-open security hole, because it is trivial to request such an URL and trigger the buffer overflow, an exploit will be the same be it 1Mb or 10Mb or any other fixed amount (I'm talking about a situation when the length of input is not checked against this buffer). The CVE# request was not about overflowing index[10], which is administrator-controlled (you actually have to write a regex that triggers it, and it will almost always cause freeswitch to crash). It was about the other overflow, the buffer for the replacement string, which is _externally_ controllable. Basically, any configuration that uses unconstrained by length regex grouping (\d+ instead of \d{1,10}) has a high potential of being vulnerable - again, from outside! - due to this. And while this is indeed an "obscure thing that nobody does", it is enough to exploit the server and take control over it. And in this context it stops being "obscure thing" and becomes a plain security hole, and this is exactly what I'm talking about. Speaking of the wording - can you imagine _my_ situation? It is the _first_ time I _ever_ looked at the software. And in the _first_ function I saw I found 3 buffer overflows. Maybe I'm just this "lucky" and there's no other bugs in there (I know that bugs are always there, but let's pretend I don't know that for clarity), so I found all remaining 3 bugs in this nice software. But it is very hard to believe that. And my reaction in this context is very much predictable, it is as difficult to think differently: "it is a buggy piece of crap, large piece". Again, please don't get me wrong here, this is just a natural reaction I'm trying to get rid of, but unfortunately it _is_ the first natural reaction that comes. If the authors lets themselves do several of such stupid bugs in a simple function, this, in my dictionary, means that they either don't think or don't know. Again, maybe this only function were written while being drunk, and all the rest are fine, but it is also difficult to believe. The question that pops up is -- what other bugs are there? Is it _all_ like this? Do you see what I mean? Sure bugs will be everywhere. But there are different bugs, and there are different _amounts_ of bugs in one simple place. And when one see a simple piece of code which contains 3 stupid bugs at once, one really questions sanity of the authors, this is the natural reaction which is very difficult to get rid of. I _am_ trying to get rid of it, really, as my time permits me. That's exactly the reason why I've tried to fix the issues, instead of just moving on. And yes, I was in rather a bad mood after all this, and wrote the CVE# request in that mood too. But still, speaking of the wording - looking at it again, I don't see where the wording coult have been much better. Yes it is possible to omit the introductional part where I describe it is my first attempt at its usage -- that's 2 first paragraphs excluding the "Hello" line, -- it is true that it is my first time with freeswitch, but it is not relevant. All the rest are pure technical details describing the issues, with rather regular technical wording. I don't see where it can be improved to be much better. Thank you!
        Hide
        Michael Tokarev added a comment -
        This become CVE-2013-2238 . Unfortunately the fixes are all mixed up together, and the ticket is now misnamed. There should have been 3 separate changes - one for the original RFE (${N} syntax), one for the index[] overflow and one for the two cases of substitution buffer overflows (CVE-2013-2238 is actually about the latter). But it's too late I guess.
        Show
        Michael Tokarev added a comment - This become CVE-2013-2238 . Unfortunately the fixes are all mixed up together, and the ticket is now misnamed. There should have been 3 separate changes - one for the original RFE (${N} syntax), one for the index[] overflow and one for the two cases of substitution buffer overflows (CVE-2013-2238 is actually about the latter). But it's too late I guess.
        Hide
        Auto Admin added a comment -
        Due to a long period of inactivity (13 or more days), this issue is due to be automatically close within 24 hours.
        If this issue is not actually resolved, please reopen it and add appropriate comments to help developers fix the issue.

        Thanks,
          Jira Admin
        Show
        Auto Admin added a comment - Due to a long period of inactivity (13 or more days), this issue is due to be automatically close within 24 hours. If this issue is not actually resolved, please reopen it and add appropriate comments to help developers fix the issue. Thanks,   Jira Admin
        Hide
        Auto Admin added a comment -
        Since there has been no change to the status of this issue, it is being closed for inactivity

        Thanks,
          Jira Admin
        Show
        Auto Admin added a comment - Since there has been no change to the status of this issue, it is being closed for inactivity Thanks,   Jira Admin

          People

          • Assignee:
            Anthony Minessale II
            Reporter:
            Matteo Brancaleoni
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development