Comments (9)
fcartegnie
commented on June 26, 2024
2
This commit also affects DAB+ channels with PS: Opendigitalradio/dablin#61
It takes some frames (e.g. 20) until
ps_datais present as part of an AAC frame. When this frame is then tried to be decoded, the decoding fails.
Yes, because DMB is exclusively PS based.
I'm working on a real fix.
from faad2.
hlef
commented on June 26, 2024
Reproducible on the latest master. I'm taking a look at it.
This was assigned CVE-2018-20360.
from faad2.
hlef
commented on June 26, 2024
This is a complicated problem. I'll try to summarize my current understanding of the issue:
The first frame has 15 channels, each channel has exactly one output channel.
The second frame still has 15 channels, but the first frame channel suddenly uses parametric stereo (PS). As a result, there are now 16 output channels, and the first frame channel has now two outputs. This is handled by the following code, in reconstruct_single_channel:
913 if (hDecoder->element_output_channels[hDecoder->fr_ch_ele] == 0)
914 {
915 /* element_output_channels not set yet */
916 hDecoder->element_output_channels[hDecoder->fr_ch_ele] = output_channels;
917 } else if (hDecoder->element_output_channels[hDecoder->fr_ch_ele] != output_channels) {
918 /* element inconsistency */
919
920 /* this only happens if PS is actually found but not in the first frame
921 * this means that there is only 1 bitstream element!
922 */
923
924 /* reset the allocation */
925 hDecoder->element_alloced[hDecoder->fr_ch_ele] = 0;
926
927 hDecoder->element_output_channels[hDecoder->fr_ch_ele] = output_channels;
928
929 //return 21;
930 }
Frame channel 1 has two outputs, so we set hDecoder->element_output_channels[1] to 2.
Now, there is a problem.
At the end of decode_sce_lfe we call
372 hDecoder->fr_channels += hDecoder->element_output_channels[hDecoder->fr_ch_ele];
373 hDecoder->fr_ch_ele++;
This means that, once we have arrived to frame channel 14, hDecoder->fr_ch_ele is 14 but hDecoder->fr_channels is 15.
Because frame channel 1 has two output channels, output channel 14 actually belongs to frame channel 13. Frame channel 14's output channel is output channel 15. This channel is not allocated.
Why? We don't allocate output channel 15 because hDecoder->element_alloced[14] is 1.
We did allocation while processing the first frame, which only had 15 output channels.
In short:
- Parametric Stereo (PS) can arrive at any moment in the file.
- The current FAAD2 code tries to handle it, fails badly.
from faad2.
hlef
commented on June 26, 2024
@fabiangreffrath I'd like to hear your thoughts about this, but I don't think I'll have time to really fix this. I don't even know whether this situation is legal or not.
I'd just remove this "feature" which is broken anyways.
diff --git a/libfaad/specrec.c b/libfaad/specrec.c
index 9797d6e..d6fdb35 100644
--- a/libfaad/specrec.c
+++ b/libfaad/specrec.c
@@ -917,16 +917,7 @@ uint8_t reconstruct_single_channel(NeAACDecStruct *hDecoder, ic_stream *ics,
} else if (hDecoder->element_output_channels[hDecoder->fr_ch_ele] != output_channels) {
/* element inconsistency */
- /* this only happens if PS is actually found but not in the first frame
- * this means that there is only 1 bitstream element!
- */
-
- /* reset the allocation */
- hDecoder->element_alloced[hDecoder->fr_ch_ele] = 0;
-
- hDecoder->element_output_channels[hDecoder->fr_ch_ele] = output_channels;
-
- //return 21;
+ return 21;
}
if (hDecoder->element_alloced[hDecoder->fr_ch_ele] == 0)
Second alternative could be:
diff --git a/libfaad/specrec.c b/libfaad/specrec.c
index 9797d6e..d07f86b 100644
--- a/libfaad/specrec.c
+++ b/libfaad/specrec.c
@@ -915,18 +915,18 @@ uint8_t reconstruct_single_channel(NeAACDecStruct *hDecoder, ic_stream *ics,
/* element_output_channels not set yet */
hDecoder->element_output_channels[hDecoder->fr_ch_ele] = output_channels;
} else if (hDecoder->element_output_channels[hDecoder->fr_ch_ele] != output_channels) {
- /* element inconsistency */
-
- /* this only happens if PS is actually found but not in the first frame
+ /* element inconsistency
+ * this only happens if PS is actually found but not in the first frame
* this means that there is only 1 bitstream element!
*/
- /* reset the allocation */
- hDecoder->element_alloced[hDecoder->fr_ch_ele] = 0;
-
- hDecoder->element_output_channels[hDecoder->fr_ch_ele] = output_channels;
-
- //return 21;
+ if (hDecoder->fr_channels == 1) {
+ /* reset the allocation */
+ hDecoder->element_alloced[hDecoder->fr_ch_ele] = 0;
+ hDecoder->element_output_channels[hDecoder->fr_ch_ele] = output_channels;
+ } else {
+ return 21;
+ }
}
if (hDecoder->element_alloced[hDecoder->fr_ch_ele] == 0)
from faad2.
fabiangreffrath
commented on June 26, 2024
The question is how often this feature is used and thus how many actual tracks are affected by this. Anyway, it is always better to return cleanly when encountering this than pretending to support it and then falling on the face. I like the second approach more, because it does at least something, but I'll leave it up to you which one to use for a PR.
from faad2.
fcartegnie
commented on June 26, 2024
This breaks with legitimate PS streams where the channel number is "unknown" before PS. (fr_channels == 0)
from faad2.
fabiangreffrath
commented on June 26, 2024
This breaks with legitimate PS streams where the channel number is "unknown" before PS. (fr_channels == 0)
Maybe this is the missing hint @hlef ?
from faad2.
basicmaster
commented on June 26, 2024
This commit also affects DAB+ channels with PS: Opendigitalradio/dablin#61
It takes some frames (e.g. 20) until ps_data is present as part of an AAC frame. When this frame is then tried to be decoded, the decoding fails.
from faad2.
fcartegnie
commented on June 26, 2024
See #51 which should be sufficient.
The real issue being dereferencing null pointer structs because we're off by one after PS.
from faad2.
Related Issues (20)
- Perpare for the next release HOT 27
- Fff
- F HOT 1
- File decoding fails
- libfaad2.dll mingw HOT 6
- Build failure with -Werror=strict-aliasing: ic_predict.c:58:16: error: dereferencing type-punned pointer will break strict-aliasing rules HOT 3
- XMMS plugin (xmms-mp4) HOT 3
- A heap-buffer-overflow vulnerability found in mp4read.c:449:63 HOT 1
- mp4read.c::u32in HOT 1
- A SEGV vulnerability found in faad2 HOT 4
- ADTS aac is not recognized HOT 2
- CMake script does not build dynamic libraries HOT 1
- man page not installed
- error: conflicting types for 'lrintf' HOT 2
- Different library versions in cmake and autotools builds HOT 6
- Builds on Debian i386 fail despite applying commit linked in #188 HOT 1
- Values begin, end variables in pns.c may not be correct in the case of short window sequences
- Incorrect table boundaries checks in huffmanc.c HOT 5
- TNS_MAX_ORDER is object type and short/long window dependent HOT 3
- TODO: fix a bunch of "warning C4711: function selected for automatic inline expansion"
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from faad2.