Bug#687597: openslp-dfsg: touch bug CVE-2012-4428

[Guillem Jover <guillem@debian.org>]

Control: severity -1 important

Hi!

On Sun, 2013-01-27 at 11:21:32 +0000, Steve McIntyre wrote:
> severity 687597 important
> thanks

(Didn't seem to take effect, I assume missing control@b.d.o Bcc.)

> On Sat, Jan 05, 2013 at 09:01:45PM +0100, John Paul Adrian Glaubitz wrote:
> > there has also been an upstream bug report filed [1].
> >
> > Might be reasonable to check back there from time to time. No patch
> > yet, unfortunately.
> 
> I had a look at this yesterday. The buffer-handling in libslp *looks*
> suspect to me (in terms of tracking lengths of text fields etc.), but
> I can't see an easy way to reproduce the bug here to verify my
> suspicions. I've followed up on the upstream bug to ask about this.
> 
> In the meantime, even if the code looks dodgy I *don't* see it as
> being particularly likely to be exploitable, more a DoS at worst, and
> only on a local-network basis rather than truly remote. I'm dropping
> severity from grave accordingly - feel free to re-raise if you think
> I'm wrong.

I was preparing a QA upload, and took a stab at this. Here's the patch
I'm going to include. It seems pretty clear that if the previous to last
character in the string-list is '\\' then the string-list handling
functions will keep going, when they probably should only have done so
on escaped ','.

Although I've only code-stared at the issue, and my later few attempts
to reproduce this have been unsuccessful, but I've to confess I've not
tried very hard. Given this I'm a bit hesitant to close this bug with
the upload, but I guess I'll do so if I don't hear complains, in a
couple of days. :)

If any of you could either test or review this, that would be much
appreciated!

Thanks,
Guillem

Description: Fix out-of-bounds buffer access (CVE-2012-4428)
 Fix handling of string-list in common/slp_common.c by not increasing
 the item pointer past the string-list pointer, and letting '\\' only
 escape the item separator ','.
Author: Guillem Jover <guillem@debian.org>
Origin: vendor
Bug: http://sourceforge.net/p/openslp/bugs/122/
Bug-Debian: https://bugs.debian.org/687597
Last-Update: 2014-07-25

---
 common/slp_compare.c |   33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

--- a/common/slp_compare.c
+++ b/common/slp_compare.c
@@ -272,13 +272,10 @@ int SLPContainsStringList(int listlen,
         /* seek to the end of the next list item */
         while(1)
         {
-            if(itemend == listend || *itemend == ',')
-            {
-                if(*(itemend - 1) != '\\')
-                {
-                    break;
-                }
-            }
+            if(itemend == listend)
+                break;
+            if(*itemend == ',' && *(itemend - 1) != '\\')
+                break;
 
             itemend ++;
         }
@@ -328,13 +325,10 @@ int SLPIntersectStringList(int list1len,
         /* seek to the end of the next list item */
         while(1)
         {
-            if(itemend == listend || *itemend == ',')
-            {
-                if(*(itemend - 1) != '\\')
-                {
-                    break;
-                }
-            }
+            if(itemend == listend)
+                break;
+            if(*itemend == ',' && *(itemend - 1) != '\\')
+                break;
 
             itemend ++;
         }
@@ -417,13 +411,10 @@ int SLPUnionStringList(int list1len,
         /* seek to the end of the next list item */
         while(1)
         {
-            if(itemend == listend || *itemend == ',')
-            {
-                if(*(itemend - 1) != '\\')
-                {
-                    break;
-                }
-            }
+            if(itemend == listend)
+                break;
+            if(*itemend == ',' && *(itemend - 1) != '\\')
+                break;
 
             itemend ++;
         }