rpms
/
sblim-sfcb
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix indCIMXmlHandler crash in IndCIMXMLHandlerInvokeMethod with Embedded Instances

This commit is contained in:
Vitezslav Crhonek 2013-05-20 12:33:42 +02:00
parent 3a8acf6b3b
commit c80186e964
4 changed files with 217 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

103
sblim-sfcb-1.3.16-embedded-crash.patch Normal file
View File

@ -0,0 +1,103 @@
replaceClStringN() relocates items in ClStrBuf, using strlen() to get
lengths of the items. With embedded instances / objects, strlen() is not
useful at all, we must get length of the item using other means.
The patch is quite crude, there might be better way, how to get the lenghts
(e.g. by having ClStrBuf.lenPtr[], which would track lenght of each item).
Version 2: do not get lengths from fb->indexPtr, it is being modified,
use a copy of it instead.
diff -up sblim-sfcb-1.3.16/objectImpl.c.embedded-crash sblim-sfcb-1.3.16/objectImpl.c
--- sblim-sfcb-1.3.16/objectImpl.c.embedded-crash 2013-04-18 16:49:51.941521957 +0200
+++ sblim-sfcb-1.3.16/objectImpl.c 2013-04-18 16:39:30.000000000 +0200
@@ -449,6 +449,29 @@ static void replaceClString(ClObjectHdr
return replaceClStringN(hdr, id, str, 0);
}
+static int getBufIndexLen(int *indexPtr, int bUsed, int iUsed, int index)
+{
+ /*
+ * Find length of 'string' in fb at given index.
+ * We cannot use strlen, because some items are not strings but embedded
+ * instances.
+ * We cannot simply substract fb->indexPtr[index+1] - fb->indexPtr[index],
+ * because the entries are not consecutive! They are shuffled by
+ * replaceClStringN().
+ * Therefore the only way to find a length of our 'string' is to find
+ * string, which starts immediately after it. Let's call it 'nearest'
+ * string. */
+ int nearest_start = bUsed;
+ int our_start = indexPtr[index];
+ int i;
+ for (i = 0; i<iUsed; i++)
+ if (indexPtr[i] > our_start && indexPtr[i] < nearest_start) {
+ nearest_start = indexPtr[i];
+ }
+ int len = nearest_start - our_start;
+ return len;
+}
+
static void
replaceClStringN(ClObjectHdr * hdr, int id, const char *str, unsigned int length)
{
@@ -457,15 +480,20 @@ replaceClStringN(ClObjectHdr * hdr, int
char *ts, *fs;
long i, l, u;
ClStrBuf *fb;
+ int *oldIndexPtr;
fb = getStrBufPtr(hdr);
ts = (char *) malloc(fb->bUsed);
fs = &fb->buf[0];
+ /* Copy indexPtr from the buffer, so we can compute lengths of items in it.*/
+ oldIndexPtr = (int*) malloc(sizeof(int)*fb->iUsed);
+ memcpy(oldIndexPtr, fb->indexPtr, sizeof(int)*fb->iUsed);
+
for (u = i = 0; i < fb->iUsed; i++) {
if (i != id - 1) {
char *f = fs + fb->indexPtr[i];
- l = strlen(f) + 1;
+ l = getBufIndexLen(oldIndexPtr, fb->bUsed, fb->iUsed, i);
fb->indexPtr[i] = u;
memcpy(ts + u, f, l);
u += l;
@@ -474,6 +502,7 @@ replaceClStringN(ClObjectHdr * hdr, int
memcpy(fs, ts, u);
fb->bUsed = u;
free(ts);
+ free(oldIndexPtr);
i = addClStringN(hdr, str, length);
fb = getStrBufPtr(hdr);
@@ -498,16 +527,20 @@ removeClObject(ClObjectHdr * hdr, int id
// char *tmpstr = NULL;
long i, l, u;
ClStrBuf *fb;
+ int *oldIndexPtr;
fb = getStrBufPtr(hdr);
ts = (char *) malloc(fb->bUsed); /* tmp string buffer */
fs = &fb->buf[0];
+ /* Copy indexPtr from the buffer, so we can compute lengths of items in it.*/
+ oldIndexPtr = (int*) malloc(sizeof(int)*fb->iUsed);
+ memcpy(oldIndexPtr, fb->indexPtr, sizeof(int)*fb->iUsed);
for (u = i = 0; i < fb->iUsed; i++) {
if (i != id - 1) { /* loop through and copy over all _other_ properties */
// fprintf(stderr, "replace: keeping %ld\n", i);
char *f = fs + fb->indexPtr[i];
- l = fb->indexPtr[i+1] - fb->indexPtr[i];
+ l = getBufIndexLen(oldIndexPtr, fb->bUsed, fb->iUsed, i);
/* Bugzilla 74159 - Align the string buffer & null terminate */
/*if (l % sizeof(long) != 0) {
@@ -535,6 +568,7 @@ removeClObject(ClObjectHdr * hdr, int id
memcpy(fs, ts, u);
fb->bUsed = u;
free(ts);
+ free(oldIndexPtr);
fb->iUsed--; /* fixup the item count, since we have one fewer elements */

76
sblim-sfcb-1.3.16-invalid-read.patch Normal file
View File

@ -0,0 +1,76 @@
Function copyStringBuf() uses sizeof(*fb->indexPtr) as size of elements
in fb->indexPtr, while addClStringN() usess 'sizeof(long)' for the same
elements. Both functions copy indexPtr, but each with different size.
Therefore, if addClStringN() is called after copyStringBuf(), it may copy more
bytes than copyStringBuf() created -> SIGSEGV (or 'Invalid read of size XYZ'
in Valgrind logs).
'sizeof(*buf->indexPtr)' should be consistently used in ClStrBuf.indexPtr
and ClArrayBuf.indexPtr.
diff -up sblim-sfcb-1.3.16/objectImpl.c.invalid-read sblim-sfcb-1.3.16/objectImpl.c
--- sblim-sfcb-1.3.16/objectImpl.c.invalid-read 2013-04-19 14:03:04.920602183 +0200
+++ sblim-sfcb-1.3.16/objectImpl.c 2013-04-19 14:04:10.229391267 +0200
@@ -208,7 +208,7 @@ addClStringN(ClObjectHdr * hdr, const ch
buf->bMax = nmax;
buf->bUsed = buf->iUsed = 0;
buf->iMax = 16;
- setStrIndexPtr(buf, malloc(sizeof(long) * 16));
+ setStrIndexPtr(buf, malloc(sizeof(*buf->indexPtr) * 16));
hdr->flags |= HDR_Rebuild;
}
@@ -222,17 +222,17 @@ addClStringN(ClObjectHdr * hdr, const ch
if (!isMallocedStrIndex(buf)) {
void *idx = buf->indexPtr;
buf->iMax = nmax * 2;
- setStrIndexPtr(buf, malloc(buf->iMax * sizeof(long)));
- memcpy(buf->indexPtr, idx, nmax * sizeof(long));
+ setStrIndexPtr(buf, malloc(buf->iMax * sizeof(*buf->indexPtr)));
+ memcpy(buf->indexPtr, idx, nmax * sizeof(*buf->indexPtr));
}
else {
buf->iMax = nmax * 2;
- setStrIndexPtr(buf, realloc(buf->indexPtr, buf->iMax * sizeof(long)));
+ setStrIndexPtr(buf, realloc(buf->indexPtr, buf->iMax * sizeof(*buf->indexPtr)));
}
}
else {
buf->iMax = 16;
- setStrIndexPtr(buf, malloc(buf->iMax * sizeof(long)));
+ setStrIndexPtr(buf, malloc(buf->iMax * sizeof(*buf->indexPtr)));
}
hdr->flags |= HDR_Rebuild;
}
@@ -289,7 +289,7 @@ static long addClArray(ClObjectHdr * hdr
buf->bMax = nmax;
buf->bUsed = buf->iUsed = 0;
buf->iMax = 16;
- setArrayIndexPtr(buf, malloc(sizeof(long) * 16));
+ setArrayIndexPtr(buf, malloc(sizeof(*buf->indexPtr) * 16));
hdr->flags |= HDR_Rebuild;
}
@@ -303,17 +303,17 @@ static long addClArray(ClObjectHdr * hdr
if (!isMallocedArrayIndex(buf)) {
void *idx = buf->indexPtr;
buf->iMax = nmax * 2;
- setArrayIndexPtr(buf, malloc(buf->iMax * sizeof(long)));
- memcpy(buf->indexPtr, idx, nmax * sizeof(long));
+ setArrayIndexPtr(buf, malloc(buf->iMax * sizeof(*buf->indexPtr)));
+ memcpy(buf->indexPtr, idx, nmax * sizeof(*buf->indexPtr));
}
else {
buf->iMax = nmax * 2;
- setArrayIndexPtr(buf, realloc(buf->indexPtr, buf->iMax * sizeof(long)));
+ setArrayIndexPtr(buf, realloc(buf->indexPtr, buf->iMax * sizeof(*buf->indexPtr)));
}
}
else {
buf->iMax = 16;
- setArrayIndexPtr(buf, malloc(buf->iMax * sizeof(long)));
+ setArrayIndexPtr(buf, malloc(buf->iMax * sizeof(*buf->indexPtr)));
}
hdr->flags |= HDR_Rebuild;
}

26
sblim-sfcb-1.3.16-invalid-read2.patch Normal file
View File

@ -0,0 +1,26 @@
The memcpy below tried to copy too much data (it's capacity of the section * 2,
max is doubled few lines above). Let's copy only the used data.
Unrelated observation:
I wonder what ensureClSpace() function does at all. How can this check
be ever true:
else if (sct->used >= max) {
'max' is basically sct->max, does that mean the sct->used contains already new
size of the section?
diff -up sblim-sfcb-1.3.16/objectImpl.c.invalid-read2 sblim-sfcb-1.3.16/objectImpl.c
--- sblim-sfcb-1.3.16/objectImpl.c.invalid-read2 2013-04-19 14:42:52.000000000 +0200
+++ sblim-sfcb-1.3.16/objectImpl.c 2013-04-19 14:43:23.039536156 +0200
@@ -168,7 +168,7 @@ static void *ensureClSpace(ClObjectHdr *
void *f,*t;
f=((char*)hdr)+sct->sectionOffset;
t=malloc(max*size);
- memcpy(t,f,max*size);
+ memcpy(t,f,sct->used*size);
sct->max=max;
setSectionPtr(sct, t);
}

13
sblim-sfcb.spec
View File

@ -8,7 +8,7 @@ Name: sblim-sfcb
Summary: Small Footprint CIM Broker
URL: http://sblim.wiki.sourceforge.net/
Version: 1.3.16
Release: 2%{?dist}
Release: 3%{?dist}
Group: Applications/System
License: EPL
Source0: http://downloads.sourceforge.net/sblim/%{name}-%{version}.tar.bz2
@ -23,6 +23,10 @@ Patch3: sblim-sfcb-1.3.14-missing-includes.patch
# Patch4: Fix provider debugging - variable for stopping wait-for-debugger
# loop must be volatile
Patch4: sblim-sfcb-1.3.15-fix-provider-debugging.patch
# Patch5-7: already upstream, http://sourceforge.net/p/sblim/sfcb-tix/37/
Patch5: sblim-sfcb-1.3.16-invalid-read.patch
Patch6: sblim-sfcb-1.3.16-invalid-read2.patch
Patch7: sblim-sfcb-1.3.16-embedded-crash.patch
Source1: sfcb.service
Provides: cim-server
Requires: cim-schema
@ -52,6 +56,9 @@ Programming Interface (CMPI).
%patch2 -p1 -b .CMGetCharPtr
%patch3 -p1 -b .missing-includes
%patch4 -p1 -b .fix-provider-debugging
%patch5 -p1 -b .invalid-read
%patch6 -p1 -b .invalid-read2
%patch7 -p1 -b .embedded-crash
%build
%configure --enable-debug --enable-uds --enable-ssl --enable-pam --enable-ipv6 CFLAGS="$CFLAGS -D_GNU_SOURCE -fPIE -DPIE" LDFLAGS="$LDFLAGS -Wl,-z,now -pie"
@ -106,6 +113,10 @@ fi;
%files -f _pkg_list
%changelog
* Mon May 20 2013 Vitezslav Crhonek <vcrhonek@redhat.com> - 1.3.16-3
- Fix indCIMXmlHandler crash in IndCIMXMLHandlerInvokeMethod with Embedded Instances
Resolves: #957747
* Tue Jan 29 2013 Vitezslav Crhonek <vcrhonek@redhat.com> - 1.3.16-2
- Fix URL in the spec file
- Remove unused devel part from the spec file