I reviewed webrtc-audio-processing version 0.1-2ubuntu2 as checked into
utopic. This review should not be considered a full security audit but
rather a quick gauge of maintainability.
- This packages WebRTC code, a collection of codec/protocols used for
realtime communication.
- Build-depends debhelper, dh-autoreconf
- Depends on nothing
- No cryptopgrahy
- Does not itself do networking
- No daemons
- No pre/post inst/rm scripts
- No init scripts
- No dbus services
- No setuid
- No binaries
- No sudo fragments
- No udev rules
- No tests are run at build time
- No cron jobs
- Build logs mostly clean, unlikely to be security-relevant
- No subprocesses are spawned
- Extensive manual memory management; many functions assumed preconditions
are met, but mostly looked safe
- Most file operations are in debugging ifdefs
- Logging looked safe
- No environment variables used
- No privileged portions of code
- No cryptography
- Does not itself do networking
- No temporary files
- No webkit
- No policykit
- Clean cppcheck, only one message, probably false positive
This code is highly technical signal processing code; it's quite possible
that codec-level flaws could have unintended consequences and we could
not possibly repair the protocol without expert assistance. On the other
hand, it looks above-average for signal processing and cleanly separates
the signal processing from other portions of code.
Here are some notes I collected while reviewing; I hope these help
someone:
WebRtcAecm_CreateCore() 16 or 32 byte aligns some members, but the
AecmCore_t structure appears to only allocate 8 or 16 bytes extra for the
buffers; if gcc is already aligning these elements on 8 byte boundaries
wouldn't that be providing 16 (sufficient) or 24 (insufficient) alignment
options? I think the neon 32-byte alignments might not be properly met.
VerifyAndAllocateFragmentationHeader() uses supplied size parameter for
memory allocations and array subscripting (in other methods), no callers
in-tree for this method? Probably needs input validation of some sort.
Why does EchoControlMobileImpl::SetEchoPath() accept a size_bytes
parameter?
Consider calloc() instead of malloc(sizeof (t) * N) to avoid integer
overflow errors. (I didn't follow all the parameters far enough to
determine if there are any exploitable conditions among these instances,
it seemed unlikely.)
Security team ACK for promoting webrtc-audio-processing to main.
Thanks
** Changed in: webrtc-audio-processing (Ubuntu)
Assignee: Seth Arnold (seth-arnold) => (unassigned)
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1325859
Title:
[MIR] webrtc-audio-processing
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/webrtc-audio-processing/+bug/1325859/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
