Buffer overflow in epicsStrnRawFromEscaped
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
EPICS Base |
Fix Released
|
Undecided
|
Unassigned |
Bug Description
Error found when Chris Ford, a member of our DAQ group ran this valgrind of caput to a UINT8 waveform
Note the missing -S, as it works with -S.
valgrind caput TST:EDT:
==9708== Memcheck, a memory error detector
==9708== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==9708== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==9708== Command: caput TST:EDT:
==9708==
==9708== Invalid write of size 1
==9708== at 0x43A226: epicsStrnRawFro
==9708== by 0x409890: main (caput.c:503)
==9708== Address 0x601aab8 is 0 bytes after a block of size 40 alloc'd
==9708== at 0x4C20140: calloc (vg_replace_
==9708== by 0x409807: main (caput.c:427)
==9708==
Old : TST:EDT:
New : TST:EDT:
==9708==
==9708== HEAP SUMMARY:
==9708== in use at exit: 20,909 bytes in 225 blocks
==9708== total heap usage: 376 allocs, 151 frees, 1,054,201 bytes allocated
==9708==
==9708== LEAK SUMMARY:
==9708== definitely lost: 430 bytes in 5 blocks
==9708== indirectly lost: 271 bytes in 1 blocks
==9708== possibly lost: 0 bytes in 0 blocks
==9708== still reachable: 20,208 bytes in 219 blocks
==9708== suppressed: 0 bytes in 0 blocks
==9708== Rerun with --leak-check=full to see details of leaked memory
==9708==
==9708== For counts of detected and suppressed errors, rerun with: -v
==9708== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 4 from 4)
Here's a patch to epicsStringTest.c that tests the bug:
--- src/libCom/
+++ src/libCom/
@@ -43,14 +43,16 @@
const char * const empty = "";
const char * const space = " ";
const char * const A = "A";
+ const char * const ABC = "ABC";
const char * const ABCD = "ABCD";
const char * const ABCDE = "ABCDE";
const char * const a = "a";
const char * const abcd = "abcd";
const char * const abcde = "abcde";
+ char result[20];
char *s;
- testPlan(281);
+ testPlan(285);
testChars();
@@ -85,5 +87,17 @@
testOk1(
testOk1(
+ /* Test for buffer overflow and NULL termination on epicsStrnEscape
+ epicsStrnEscape
+ epicsStrnEscape
+ testOk( result[
+ testOk( strcmp(result, ABC) == 0, "epicsStrnEscap
+
+ /* Test for buffer overflow and NULL termination on epicsStrnRawFro
+ epicsStrnRawFro
+ epicsStrnRawFro
+ testOk( result[
+ testOk( strcmp(result, ABC) == 0, "epicsStrnRawFr
+
return testDone();
}
Possible patch to fix it based on 3.14.12.4 epicsString.c
--- src/libCom/
+++ src/libCom/
@@ -40,6 +40,7 @@
{
const char *pfrom = from;
char *pto = to;
+ char *pend = to + outsize - 1;
char c;
int nto = 0, nfrom = 0;
@@ -100,7 +101,10 @@
*pto++ = c; nto++;
}
}
- *pto = '\0'; /* NOTE that nto does not have to be incremented */
+ /* Guard against writing past the end of the buffer */
+ if( pto > pend )
+ pto = pend;
+ *pto = '\0'; /* NOTE that nto does not have to be incremented */
return nto;
}
Related branches
Changed in epics-base: | |
status: | Fix Committed → Fix Released |
Also in 3.15 series