Fixing Coverity issues in Audio Coding Module

10198: Out-of-bounds read in acm_isac.cc
10251: Unintended sign extension in acm_resampler.cc
10273: Uninitialized pointer field in acm_amr.cc
10274: Uninitialized pointer field in acm_amrwb.cc
10275: Uninitialized scalar field in acm_dtmf_detection.cc
10276: Uninitialized pointer field in acm_g722.cc
10277: Uninitialized pointer field in acm_g7221.cc
10278: Uninitialized pointer field in acm_g7221c.cc
10279: Uninitialized pointer field in acm_g729.cc
10280: Uninitialized pointer field in acm_g7291.cc
10281: Uninitialized pointer scalar in acm_generic_codec.cc
10282: Uninitialized pointer field in acm_gsmfr.cc
10283: Uninitialized scalar field in acm_isac.cc
10284: Uninitialized pointer field in acm_opus.cc
10285: Uninitialized scalar field in acm_resampler.cc
10286: Uninitialized pointer field in acm_speex.cc
10287: Uninitialized scalar field in audio_coding_module_impl.cc
10581: Unintended sign extension in audio_coding_module_impl.cc

Additional change: removed unused function and member from ACMResampler.

Review URL: https://webrtc-codereview.appspot.com/343016

git-svn-id: http://webrtc.googlecode.com/svn/trunk@1471 4adac7df-926f-26a2-2b94-8c16560cd09d

src/modules/audio_coding/main/source/acm_amr.cc

@@ -50,7 +50,13 @@
 namespace webrtc {
 
 #ifndef WEBRTC_CODEC_AMR
-ACMAMR::ACMAMR(WebRtc_Word16 /* codecID */) {
+ACMAMR::ACMAMR(WebRtc_Word16 /* codecID */)
+    : _encoderInstPtr(NULL),
+      _decoderInstPtr(NULL),
+      _encodingMode(-1),  // Invalid value.
+      _encodingRate(0),  // Invalid value.
+      _encoderPackingFormat(AMRBandwidthEfficient),
+      _decoderPackingFormat(AMRBandwidthEfficient) {
   return;
 }
 

src/modules/audio_coding/main/source/acm_amrwb.cc

@@ -47,7 +47,13 @@
 namespace webrtc {
 
 #ifndef WEBRTC_CODEC_AMRWB
-ACMAMRwb::ACMAMRwb(WebRtc_Word16 /* codecID*/) {
+ACMAMRwb::ACMAMRwb(WebRtc_Word16 /* codecID*/)
+    : _encoderInstPtr(NULL),
+      _decoderInstPtr(NULL),
+      _encodingMode(-1),  // invalid value
+      _encodingRate(0),  // invalid value
+      _encoderPackingFormat(AMRBandwidthEfficient),
+      _decoderPackingFormat(AMRBandwidthEfficient) {
   return;
 }
 

src/modules/audio_coding/main/source/acm_dtmf_detection.cc

@@ -13,7 +13,8 @@
 
 namespace webrtc {
 
-ACMDTMFDetection::ACMDTMFDetection() {}
+ACMDTMFDetection::ACMDTMFDetection()
+    : _init(0) {}
 
 ACMDTMFDetection::~ACMDTMFDetection() {}
 

src/modules/audio_coding/main/source/acm_g722.cc

@@ -21,7 +21,12 @@ namespace webrtc {
 
 #ifndef WEBRTC_CODEC_G722
 
-ACMG722::ACMG722(WebRtc_Word16 /* codecID */) {
+ACMG722::ACMG722(WebRtc_Word16 /* codecID */)
+    : _ptrEncStr(NULL),
+      _ptrDecStr(NULL),
+      _encoderInstPtr(NULL),
+      _encoderInstPtrRight(NULL),
+      _decoderInstPtr(NULL) {
   return;
 }
 
@@ -106,7 +111,10 @@ struct ACMG722DecStr {
   G722DecInst* instRight; // instance for right channel in case of stereo
 };
 
-ACMG722::ACMG722(WebRtc_Word16 codecID) {
+ACMG722::ACMG722(WebRtc_Word16 codecID)
+    : _encoderInstPtr(NULL),
+      _encoderInstPtrRight(NULL),
+      _decoderInstPtr(NULL) {
   // Encoder
   _ptrEncStr = new ACMG722EncStr;
   if (_ptrEncStr != NULL) {

src/modules/audio_coding/main/source/acm_g7221.cc

@@ -88,7 +88,20 @@ namespace webrtc {
 
 #ifndef WEBRTC_CODEC_G722_1
 
-ACMG722_1::ACMG722_1(WebRtc_Word16 /* codecID */) {
+ACMG722_1::ACMG722_1(WebRtc_Word16 /* codecID */)
+    : _operationalRate(-1),
+      _encoderInstPtr(NULL),
+      _encoderInstPtrRight(NULL),
+      _decoderInstPtr(NULL),
+      _encoderInst16Ptr(NULL),
+      _encoderInst16PtrR(NULL),
+      _encoderInst24Ptr(NULL),
+      _encoderInst24PtrR(NULL),
+      _encoderInst32Ptr(NULL),
+      _encoderInst32PtrR(NULL),
+      _decoderInst16Ptr(NULL),
+      _decoderInst24Ptr(NULL),
+      _decoderInst32Ptr(NULL) {
   return;
 }
 

src/modules/audio_coding/main/source/acm_g7221c.cc

@@ -88,8 +88,20 @@ namespace webrtc {
 
 #ifndef WEBRTC_CODEC_G722_1C
 
-ACMG722_1C::ACMG722_1C(
+ACMG722_1C::ACMG722_1C(WebRtc_Word16 /* codecID */)
-    WebRtc_Word16 /* codecID */) {
+    : _operationalRate(-1),
+      _encoderInstPtr(NULL),
+      _encoderInstPtrRight(NULL),
+      _decoderInstPtr(NULL),
+      _encoderInst24Ptr(NULL),
+      _encoderInst24PtrR(NULL),
+      _encoderInst32Ptr(NULL),
+      _encoderInst32PtrR(NULL),
+      _encoderInst48Ptr(NULL),
+      _encoderInst48PtrR(NULL),
+      _decoderInst24Ptr(NULL),
+      _decoderInst32Ptr(NULL),
+      _decoderInst48Ptr(NULL) {
   return;
 }
 

src/modules/audio_coding/main/source/acm_g729.cc

@@ -42,10 +42,10 @@ namespace webrtc {
 
 #ifndef WEBRTC_CODEC_G729
 
-ACMG729::ACMG729(
+ACMG729::ACMG729(WebRtc_Word16 /* codecID */)
-    WebRtc_Word16 /* codecID */)
+    : _encoderInstPtr(NULL),
-{
+      _decoderInstPtr(NULL) {
-    return;
+  return;
 }
 
 

src/modules/audio_coding/main/source/acm_g7291.cc

@@ -42,10 +42,10 @@ namespace webrtc {
 
 #ifndef WEBRTC_CODEC_G729_1
 
-ACMG729_1::ACMG729_1(
+ACMG729_1::ACMG729_1( WebRtc_Word16 /* codecID */)
-    WebRtc_Word16 /* codecID */)
+    : _encoderInstPtr(NULL),
-{
+      _decoderInstPtr(NULL) {
-    return;
+  return;
 }
 
 

src/modules/audio_coding/main/source/acm_generic_codec.cc

@@ -40,9 +40,9 @@ ACMGenericCodec::ACMGenericCodec()
       _inTimestampIxWrite(0),
       _inAudio(NULL),
       _inTimestamp(NULL),
-      _frameLenSmpl(-1),    // invalid value
+      _frameLenSmpl(-1),  // invalid value
       _noChannels(1),
-      _codecID(-1), // invalid value
+      _codecID(-1),  // invalid value
       _noMissedSamples(0),
       _encoderExist(false),
       _decoderExist(false),
@@ -61,12 +61,22 @@ ACMGenericCodec::ACMGenericCodec()
       _netEqDecodeLock(NULL),
       _codecWrapperLock(*RWLockWrapper::CreateRWLock()),
       _lastEncodedTimestamp(0),
-      _lastTimestamp(0),
+      _lastTimestamp(0xD87F3F9F),
       _isAudioBuffFresh(true),
       _uniqueID(0) {
-  _lastTimestamp = 0xD87F3F9F;
+  // Initialize VAD vector.
-  //NullifyCodecInstance();
+  for (int i = 0; i < MAX_FRAME_SIZE_10MSEC; i++) {
+    _vadLabel[i] = 0;
+  }
+
+  // Nullify memory for encoder and decoder, and set payload type to an
+  // invalid value.
+  memset(&_encoderParams, 0, sizeof(WebRtcACMCodecParams));
+  _encoderParams.codecInstant.pltype = -1;
+  memset(&_decoderParams, 0, sizeof(WebRtcACMCodecParams));
+  _decoderParams.codecInstant.pltype = -1;
 }
+
 ACMGenericCodec::~ACMGenericCodec()
 {
     // Check all the members which are pointers and

src/modules/audio_coding/main/source/acm_gsmfr.cc

@@ -42,10 +42,10 @@ namespace webrtc {
 
 #ifndef WEBRTC_CODEC_GSMFR
 
-ACMGSMFR::ACMGSMFR(
+ACMGSMFR::ACMGSMFR(WebRtc_Word16 /* codecID */)
-    WebRtc_Word16 /* codecID */)
+    : _encoderInstPtr(NULL),
-{
+      _decoderInstPtr(NULL) {
-    return;
+  return;
 }
 
 

src/modules/audio_coding/main/source/acm_ilbc.cc

@@ -24,10 +24,10 @@ namespace webrtc
 
 #ifndef WEBRTC_CODEC_ILBC
 
-ACMILBC::ACMILBC(
+ACMILBC::ACMILBC(WebRtc_Word16 /* codecID */)
-    WebRtc_Word16 /* codecID */)
+    : _encoderInstPtr(NULL),
-{
+      _decoderInstPtr(NULL) {
-    return;
+  return;
 }
 
 

src/modules/audio_coding/main/source/acm_isac.cc

@@ -86,10 +86,18 @@ const WebRtc_Word32 isacRatesSWB[NR_ISAC_BANDWIDTHS] =
 
 #if (!defined(WEBRTC_CODEC_ISAC) && !defined(WEBRTC_CODEC_ISACFX))
 
-ACMISAC::ACMISAC(
+ACMISAC::ACMISAC(WebRtc_Word16 /* codecID */)
-    WebRtc_Word16 /* codecID */)
+    : _codecInstPtr(NULL),
-{
+      _isEncInitialized(false),
-    return;
+      _isacCodingMode(CHANNEL_INDEPENDENT),
+      _enforceFrameSize(false),
+      _isacCurrentBN(32000),
+      _samplesIn10MsAudio(160) {  // Initiates to 16 kHz mode.
+  // Initiate decoder parameters for the 32 kHz mode.
+  memset(&_decoderParams32kHz, 0, sizeof(WebRtcACMCodecParams));
+  _decoderParams32kHz.codecInstant.pltype = -1;
+
+  return;
 }
 
 
@@ -453,24 +461,29 @@ ACMISACFixGetDecSampRate(
 
 
 
-ACMISAC::ACMISAC(
+ACMISAC::ACMISAC(WebRtc_Word16 codecID)
-    WebRtc_Word16 codecID):
+    : _isEncInitialized(false),
-_codecInstPtr(NULL)
+      _isacCodingMode(CHANNEL_INDEPENDENT),
-{
+      _enforceFrameSize(false),
-    _codecInstPtr = new ACMISACInst;
+      _isacCurrentBN(32000),
-    if (_codecInstPtr == NULL)
+      _samplesIn10MsAudio(160) {  // Initiates to 16 kHz mode.
-    {
+  _codecID = codecID;
-        return;
-    }
-    _codecInstPtr->inst = NULL;
-    _codecID = codecID;
-    _enforceFrameSize = false;
-    // by default a 16 kHz iSAC is created.
-    _samplesIn10MsAudio = 160;
 
-    // Initialize values that can be used uninitialized otherwise
+  // Create codec instance.
-    _decoderParams.codecInstant.pltype = -1;
+  _codecInstPtr = new ACMISACInst;
-    _decoderParams32kHz.codecInstant.pltype = -1;
+  if (_codecInstPtr == NULL) {
+    return;
+  }
+  _codecInstPtr->inst = NULL;
+
+  // Initiate decoder parameters for the 32 kHz mode.
+  memset(&_decoderParams32kHz, 0, sizeof(WebRtcACMCodecParams));
+  _decoderParams32kHz.codecInstant.pltype = -1;
+
+  // TODO(tlegrand): Check if the following is really needed, now that
+  // ACMGenericCodec has been updated to initialize this value.
+  // Initialize values that can be used uninitialized otherwise
+  _decoderParams.codecInstant.pltype = -1;
 }
 
 
@@ -893,7 +906,7 @@ ACMISAC::GetEstimatedBandwidthSafe()
     ACM_ISAC_GETSENDBWE(_codecInstPtr->inst, &bandwidthIndex, &delayIndex);
 
     // Validy check of index
-    if ((bandwidthIndex < 0) || (bandwidthIndex > NR_ISAC_BANDWIDTHS))
+    if ((bandwidthIndex < 0) || (bandwidthIndex >= NR_ISAC_BANDWIDTHS))
     {
         return -1;
     }

src/modules/audio_coding/main/source/acm_opus.cc

@@ -26,10 +26,14 @@ namespace webrtc
 
 #ifndef WEBRTC_CODEC_OPUS
 
-ACMOPUS::ACMOPUS(
+ACMOPUS::ACMOPUS(WebRtc_Word16 /* codecID */)
-    WebRtc_Word16 /* codecID */)
+    : _encoderInstPtr(NULL),
-{
+      _decoderInstPtr(NULL),
-    return;
+      _mySampFreq(0),
+      _myRate(0),
+      _opusMode(0),
+      _flagVBR(0) {
+  return;
 }
 
 
@@ -164,25 +168,19 @@ extern WebRtc_Word16 WebRtcOpus_DecodeBwe(OPUS_inst_t_* decInst, WebRtc_Word16*
 extern WebRtc_Word16 WebRtcOpus_DecodePlc(OPUS_inst_t_* decInst);
 extern WebRtc_Word16 WebRtcOpus_DecoderInit(OPUS_inst_t_* decInst);
 
-ACMOPUS::ACMOPUS(
+ACMOPUS::ACMOPUS(WebRtc_Word16 codecID)
-    WebRtc_Word16 codecID):
+    : _encoderInstPtr(NULL),
-_encoderInstPtr(NULL),
+      _decoderInstPtr(NULL),
-_decoderInstPtr(NULL),
+      _mySampFreq(48000),  // Default sampling frequency.
-_opusMode(1),           // default mode is the hybrid mode
+      _myRate(50000),  // Default rate.
-_flagVBR(0)             // default VBR off
+      _opusMode(1),  // Default mode is the hybrid mode.
-{
+      _flagVBR(0) {  // Default VBR off.
-    _codecID = codecID;
+  _codecID = codecID;
 
-    // Current implementation doesn't have DTX. That might change.
+  // Current implementation doesn't have DTX. That might change.
-    _hasInternalDTX = false;
+  _hasInternalDTX = false;
 
-    // Default sampling frequency
+  return;
-    _mySampFreq = 48000;
-
-    // default rate
-    _myRate = 50000;
-
-    return;
 }
 
 ACMOPUS::~ACMOPUS()

src/modules/audio_coding/main/source/acm_resampler.cc

@@ -45,8 +45,9 @@ ACMResampler::Resample10Msec(
 
     if(inFreqHz == outFreqHz)
     {
-        memcpy(outAudio, inAudio, (inFreqHz*numAudioChannels / 100) * sizeof(WebRtc_Word16));
+        size_t length = static_cast<size_t>(inFreqHz * numAudioChannels / 100);
-        return (WebRtc_Word16)(inFreqHz / 100);
+        memcpy(outAudio, inAudio, length * sizeof(WebRtc_Word16));
+        return static_cast<WebRtc_Word16>(inFreqHz / 100);
     }
 
     int maxLen = 480 * numAudioChannels; //max number of samples for 10ms at 48kHz
@@ -60,7 +61,7 @@ ACMResampler::Resample10Msec(
     ret = _resampler.ResetIfNeeded(inFreqHz,outFreqHz,type);
     if (ret < 0)
     {
-        WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, _id,
+        WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, 0,
             "Error in reset of resampler");
         return -1;
     }
@@ -68,7 +69,7 @@ ACMResampler::Resample10Msec(
     ret = _resampler.Push(inAudio, lengthIn, outAudio, maxLen, outLen);
     if (ret < 0 )
     {
-        WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, _id,
+        WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, 0,
             "Error in resampler: resampler.Push");
         return -1;
     }
@@ -79,12 +80,4 @@ ACMResampler::Resample10Msec(
 
 }
 
-void
-ACMResampler::SetUniqueId(
-    WebRtc_Word32 id)
-{
-    CriticalSectionScoped lock(_resamplerCritSect);
-    _id = id;
-}
-
 } // namespace webrtc

src/modules/audio_coding/main/source/acm_resampler.h

@@ -31,14 +31,10 @@ public:
         const WebRtc_Word32  outFreqHz,
         WebRtc_UWord8        numAudioChannels);
 
-    void SetUniqueId(
-        WebRtc_Word32 id);
-
 private:
 
     //Use the Resampler class
     Resampler               _resampler;
-    WebRtc_Word32           _id;
     CriticalSectionWrapper& _resamplerCritSect;
 };
 

src/modules/audio_coding/main/source/acm_speex.cc

@@ -53,8 +53,9 @@ namespace webrtc {
 
 #ifndef WEBRTC_CODEC_SPEEX
 ACMSPEEX::ACMSPEEX(WebRtc_Word16 /* codecID*/)
-{
+    : _encoderInstPtr(NULL),
-    return;
+      _decoderInstPtr(NULL) {
+  return;
 }
 
 ACMSPEEX::~ACMSPEEX()

src/modules/audio_coding/main/source/audio_coding_module_impl.cc

@@ -65,9 +65,11 @@ AudioCodingModuleImpl::AudioCodingModuleImpl(
     _fecEnabled(false),
     _fragmentation(NULL),
     _lastFECTimestamp(0),
+    _redPayloadType(255),
     _receiveREDPayloadType(255),  // invalid value
     _previousPayloadType(255),
     _dummyRTPHeader(NULL),
+    _recvPlFrameSizeSmpls(0),
     _receiverInitialized(false),
     _dtmfDetector(NULL),
     _dtmfCallback(NULL),
@@ -76,8 +78,19 @@ AudioCodingModuleImpl::AudioCodingModuleImpl(
 {
     _lastTimestamp = 0xD87F3F9F;
     _lastInTimestamp = 0xD87F3F9F;
-    // nullify the codec name
+
+    // Nullify send codec memory, set payload type and set codec name to
+    // invalid values.
+    memset(&_sendCodecInst, 0, sizeof(CodecInst));
     strncpy(_sendCodecInst.plname, "noCodecRegistered", 31);
+    _sendCodecInst.pltype = -1;
+
+    // Nullify memory for CNG, DTMF and RED.
+    memset(&_cngNB, 0, sizeof(CodecInst));
+    memset(&_cngWB, 0, sizeof(CodecInst));
+    memset(&_cngSWB, 0, sizeof(CodecInst));
+    memset(&_RED, 0, sizeof(CodecInst));
+    memset(&_DTMF, 0, sizeof(CodecInst));
 
     for (int i = 0; i < ACMCodecDB::kMaxNumCodecs; i++)
     {
@@ -1177,9 +1190,10 @@ match");
       }
     } else {
       // Copy payload data for future use.
-      memcpy(audio, audioFrame._payloadData,
+      size_t length = static_cast<size_t>(
-             audioFrame._payloadDataLengthInSamples * audio_channels *
+          audioFrame._payloadDataLengthInSamples * audio_channels *
-             sizeof(WebRtc_UWord16));
+          sizeof(WebRtc_UWord16));
+      memcpy(audio, audioFrame._payloadData, length);
     }
 
     WebRtc_UWord32 currentTimestamp;

src/modules/audio_coding/main/source/audio_coding_module_impl.h

@@ -315,9 +315,7 @@ private:
     CodecInst                      _cngWB;
     CodecInst                      _cngSWB;
     CodecInst                      _RED;
-    bool                           _REDRegistered;
     CodecInst                      _DTMF;
-    bool                           _DTMFRegistered;
     bool                           _vadEnabled;
     bool                           _dtxEnabled;
     ACMVADMode                     _vadMode;