JvanKatwijk / dab-cmdline

DAB decoding library with example of its use
GNU General Public License v2.0
57 stars 29 forks source link

Stack corruption in reedSolomon::decode_rs #73

Closed srcejon closed 3 years ago

srcejon commented 3 years ago

Hi,

It looks like there is stack corruption in reedSolomon::decode_rs().

It has:

uint8_t omega [nroots];

Then calls computeOmega()

Which does:

omega [nroots] = codeLength;

omega [nroots] is beyond the end of the allocated array (which of course only goes to nroots-1).

JvanKatwijk commented 3 years ago

That is a good one! I'll have it fixed.

Btw, I was modifying some code in the library (and some of the example programs), since I was somewhat unhappy with passing individual functions through the who;e computing chain. Change will be a (major) the dabInit and other functions take as parameter a struct API_struct with as fields the various callback functions, b (minor) I added a callback function that can be used to print the TII data (major, minor)

On the todo list is a rewrite of example 5 (the one with keyboard "up" and "down" through the list), where a simple message queui is used to handle the result of the callbacks.

Op wo 21 apr. 2021 om 22:04 schreef srcejon @.***>:

Hi,

It looks like there is stack corruption in reedSolomon::decode_rs().

It has:

uint8_t omega [nroots];

Then calls computeOmega()

Which does:

omega [nroots] = codeLength;

omega [nroots] is beyond the end of the allocated array (which of course only goes to nroots-1).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/issues/73, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQDGKMQFQ75ZYGQNMCTTJ4VUTANCNFSM43LBMVFQ .

-- Jan van Katwijk

srcejon commented 3 years ago

Great - note that in the MacOS patch you just merged. I'd increased the size of the array by 1 as a temporary workaround.

JvanKatwijk commented 3 years ago

yes I've seen that! When I find some time I'll dive into the rs code (has been a long time)

Op do 22 apr. 2021 om 14:57 schreef srcejon @.***>:

Great - note that in the MacOS patch you just merged. I'd increased the size of the array by 1 as a temporary workaround.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/issues/73#issuecomment-824817513, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQHFTTYBMMYTP6MKL4LTKAMLTANCNFSM43LBMVFQ .

-- Jan van Katwijk

JvanKatwijk commented 3 years ago

Interesting is that in qt-dab the size was adapted, similar to the size of lambda, also nroots+1

Op do 22 apr. 2021 om 15:13 schreef jan van katwijk @.***

:

yes I've seen that! When I find some time I'll dive into the rs code (has been a long time)

Op do 22 apr. 2021 om 14:57 schreef srcejon @.***>:

Great - note that in the MacOS patch you just merged. I'd increased the size of the array by 1 as a temporary workaround.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/issues/73#issuecomment-824817513, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQHFTTYBMMYTP6MKL4LTKAMLTANCNFSM43LBMVFQ .

-- Jan van Katwijk

-- Jan van Katwijk