From 7c67b7de7cee32c52e520aa540395daf9e9e9018 Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Fri, 20 Apr 2018 17:49:46 -0700 Subject: [PATCH 1/3] qrtplib: clear buffer fed to RTPSession::CreateCNAME The RTPSession::CreateCNAME function checks to see if the buffer that it is provided already has any data in it, and appends to it if so. The RTPSession::InternalCreate function calls this function with an uninitialized buffer, which results in indeterminate behavior. To ensure that the CNAME is properly created, we clear the buffer before use. ==30323== Conditional jump or move depends on uninitialised value(s) ==30323== at 0x4C30109: __strlen_sse2 (vg_replace_strmem.c:460) ==30323== by 0x85647A4: qrtplib::RTPSession::CreateCNAME(unsigned char*, unsigned long*, bool) (rtpsession.cpp:1150) ==30323== by 0x8564B35: qrtplib::RTPSession::InternalCreate(qrtplib::RTPSessionParams const&) (rtpsession.cpp:218) ==30323== by 0x5499159: RTPSink::RTPSink(QUdpSocket*, int, bool) (rtpsink.cpp:48) ==30323== by 0x5420B6A: AudioNetSink::AudioNetSink(QObject*, int, bool) (audionetsink.cpp:42) ==30323== by 0x541F465: AudioOutput::start(int, int) (audiooutput.cpp:114) ==30323== by 0x5412763: AudioDeviceManager::startAudioOutput(int) (audiodevicemanager.cpp:361) ==30323== by 0x5412B0C: AudioDeviceManager::addAudioSink(AudioFifo*, MessageQueue*, int) (audiodevicemanager.cpp:229) ==30323== by 0x33F96DE7: BFMDemod::BFMDemod(DeviceSourceAPI*) (bfmdemod.cpp:56) ==30323== by 0x33FB03F2: non-virtual thunk to BFMPlugin::createRxChannelBS(DeviceSourceAPI*) (bfmplugin.cpp:62) ==30323== by 0x4F47F25: DeviceUISet::loadRxChannelSettings(Preset const*, PluginAPI*) (deviceuiset.cpp:199) ==30323== by 0x4EA51EA: MainWindow::loadPresetSettings(Preset const*, int) (mainwindow.cpp:575) ==30323== by 0x4EAC81B: MainWindow::MainWindow(qtwebapp::LoggerWithFile*, MainParser const&, QWidget*) (mainwindow.cpp:176) ==30323== by 0x10A49B: runQtApplication(int, char**, qtwebapp::LoggerWithFile*) (main.cpp:120) ==30323== by 0x109B38: main (main.cpp:131) --- qrtplib/rtpsession.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qrtplib/rtpsession.cpp b/qrtplib/rtpsession.cpp index 643858cb8..a9d42a8d2 100644 --- a/qrtplib/rtpsession.cpp +++ b/qrtplib/rtpsession.cpp @@ -209,7 +209,7 @@ int RTPSession::InternalCreate(const RTPSessionParams &sessparams) // Init the RTCP packet builder double timestampunit = sessparams.GetOwnTimestampUnit(); - uint8_t buf[1024]; + uint8_t buf[1024] = {0}; std::size_t buflen = 1024; std::string forcedcname = sessparams.GetCNAME(); From bc4d7adce79035bd1c4e7c105f252582c20adfce Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Fri, 20 Apr 2018 21:52:57 -0700 Subject: [PATCH 2/3] FileSourceGui: Prevent potential integer overflow in updateWithStreamTime UBSan reports the following error when replaying an IQ stream: ./plugins/samplesource/filesource/filesourcegui.cpp:331:29: runtime error: signed integer overflow: 2704064 * 1000 cannot be represented in type 'int' By rearranging the calculation, we can be sure that the calculation never overflows. --- plugins/samplesource/filesource/filesourcegui.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/samplesource/filesource/filesourcegui.cpp b/plugins/samplesource/filesource/filesourcegui.cpp index d679ae889..c74a6270f 100644 --- a/plugins/samplesource/filesource/filesourcegui.cpp +++ b/plugins/samplesource/filesource/filesourcegui.cpp @@ -328,8 +328,8 @@ void FileSourceGui::updateWithStreamTime() int t_msec = 0; if (m_sampleRate > 0){ - t_msec = ((m_samplesCount * 1000) / m_sampleRate) % 1000; t_sec = m_samplesCount / m_sampleRate; + t_msec = (m_samplesCount - (t_sec * m_sampleRate)) * 1000 / m_sampleRate; } QTime t(0, 0, 0, 0); From 141997475ca5d927c1d48ca5a737b55526c85d15 Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Sat, 21 Apr 2018 09:53:04 -0700 Subject: [PATCH 3/3] BFM demod: RDS demod: Initialize RDSDemod array elements The m_parms.tot_errs array is not initialized prior to its first use in the RDSDemod::biphase function. ASAN does not pick up on this directly, but instead reports it as follows (note that ASAN fills memory with 0xBE and -1094795586 is 0xBEBEBEBE): ./plugins/channelrx/demodbfm/rdsdemod.cpp:159:95: runtime error: signed integer overflow: -1094795586 + -1094795586 cannot be represented in type 'int' The m_parms.subcarr_bb array does not appear to be read prior to initialization, but we initialize it to zero anyway for the sake of good hygiene. --- plugins/channelrx/demodbfm/rdsdemod.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/channelrx/demodbfm/rdsdemod.cpp b/plugins/channelrx/demodbfm/rdsdemod.cpp index fd5094f75..c38cccdf8 100644 --- a/plugins/channelrx/demodbfm/rdsdemod.cpp +++ b/plugins/channelrx/demodbfm/rdsdemod.cpp @@ -36,6 +36,7 @@ RDSDemod::RDSDemod() m_srate = 250000; m_parms.subcarr_phi = 0; + memset(m_parms.subcarr_bb, 0, sizeof(m_parms.subcarr_bb)); m_parms.clock_offset = 0; m_parms.clock_phi = 0; m_parms.prev_clock_phi = 0; @@ -48,6 +49,7 @@ RDSDemod::RDSDemod() m_parms.prev_acc = 0; m_parms.counter = 0; m_parms.reading_frame = 0; + memset(m_parms.tot_errs, 0, sizeof(m_parms.tot_errs)); m_parms.dbit = 0; m_prev = 0.0f; memset(m_xv, 0, 6*sizeof(Real));