Buffer overrun in dbpr with long INP field

Bug #1776141 reported by Martin Konrad on 2018-06-11
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
EPICS Base
Status tracked in 7.0
3.15
Undecided
Unassigned
3.16
Undecided
Andrew Johnson
7.0
Undecided
Andrew Johnson

Bug Description

softIoc crashes with a buffer overflow in dbTest.c:1152 when running "dbpr A 4" on the attached database file (test.db).

Root cause: pmsg points to msgBuff->message which has a fixed size of 128 but the output of sprintf can be longer.

I can see two potential solutions here:

1. Use snprintf() to prevent the buffer overflow.
2. Convert the file to C++ and use strings.

Note: dbTest.c contains a total of 23 sprintf() calls so there might be potential for more issues...

Changed in epics-base:
assignee: nobody → Martin Konrad (info-martin-konrad)
mdavidsaver (mdavidsaver) wrote :

I'm not able to replicate with current 3.16 branch (plus some extra changes, but I don't think relevant). I see an explicit error and no crash. Valgrind doesn't report a bounds violation.

epics> dbpr A 4
...
INP:CA_LINK AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
dbpr_msgOut: ERROR - msg length=149 limit=80
...

Here is what I did:

git clone https://git.launchpad.net/epics-base
cd epics-base/
git checkout 3.16
make
wget https://bugs.launchpad.net/epics-base/+bug/1776141/+attachment/5151133/+files/test.db
./bin/linux-x86_64/softIoc -D dbd/softIoc.dbd -d test.db

mdavidsaver (mdavidsaver) wrote :

As we found yesterday, the specific error you see is due to building with _FORTIFY_SOURCE checks enabled.

The print which triggers this is in the DBF_*LINK case of dbpr_report().

sprintf(pmsg,"%s:%s %s", pfield_name,
    pamaplinkType[ind].strvalue,dbGetString(pdbentry));

Specifically the "dbGetString()" call is returning a long string.

mdavidsaver (mdavidsaver) wrote :

*** buffer overflow detected ***: ./bin/linux-x86_64/softIoc terminated
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x70bfb)[0x7f8edae38bfb]
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x37)[0x7f8edaec11f7]
/lib/x86_64-linux-gnu/libc.so.6(+0xf7330)[0x7f8edaebf330]
/lib/x86_64-linux-gnu/libc.so.6(+0xf68e9)[0x7f8edaebe8e9]
/lib/x86_64-linux-gnu/libc.so.6(_IO_default_xsputn+0xac)[0x7f8edae3cc0c]
/lib/x86_64-linux-gnu/libc.so.6(_IO_vfprintf+0x1ebb)[0x7f8edae10beb]
/lib/x86_64-linux-gnu/libc.so.6(__vsprintf_chk+0x8c)[0x7f8edaebe97c]
/lib/x86_64-linux-gnu/libc.so.6(__sprintf_chk+0x7d)[0x7f8edaebe8cd]
/home/mdavidsaver/work/epics/base-git/bin/linux-x86_64/../../lib/linux-x86_64/libdbCore.so.3.16.1(dbpr+0x469)[0x7f8edbf23049]
/home/mdavidsaver/work/epics/base-git/bin/linux-x86_64/../../lib/linux-x86_64/libCom.so.3.16.1(+0x3390b)[0x7f8edba3790b]
./bin/linux-x86_64/softIoc(main+0x2be)[0x55bbb64a1c5e]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1)[0x7f8edade82e1]
./bin/linux-x86_64/softIoc(_start+0x2a)[0x55bbb64a1f7a]

mdavidsaver (mdavidsaver) wrote :

cat <<EOF > configure/os/CONFIG_SITE.Common.linux-x86_64
OP_SYS_CFLAGS += -fdebug-prefix-map=$(TOP)=. -fstack-protector-strong -Wformat -Werror=format-security
OP_SYS_CXXFLAGS += -fdebug-prefix-map=$(TOP)=. -fstack-protector-strong -Wformat -Werror=format-security
OP_SYS_CPPFLAGS += -Wdate-time -D_FORTIFY_SOURCE=2
EOF

Andrew Johnson (anj) wrote :

I have just committed some changes to the 3.16 branch which should fix this bug. I already had some changes ready that improve the output from dbpr, so I committed those, fixed the erroneous sprintf() that caused the above problem, and also fixed an incorrect output problem when printing long strings from dbgf.

Please update and test.

Changed in epics-base:
status: New → In Progress

I can confirm that Andrew's change fixed the bug.

This bug also affects Base 3.15.

Andrew Johnson (anj) wrote :

@Ralph is this important enough to hold up 3.15.6 for? That's what attaching that milestone to the bug means to me.

IIRC my fix on the 3.16 branch relied on some other changes that were already in dbTest.c so back-porting the fix to the 3.15 branch might take some work.

Ralph Lange (ralph-lange) wrote :

I admit I have lost track on where we stand wrt the next releases.

No, I don't think this would be a reason to hold up anything. Feel free to close as wontfix or retarget.

Andrew Johnson (anj) wrote :

I just untargeted 3.15.6; as Martin says the bug affects the 3.15 branch so I won't close it.

Andrew Johnson (anj) wrote :

Core Group review at ESS: Fixed in 3.16, we're open to someone else back-porting or otherwise fixing 3.15.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Bug attachments