Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Out of bounds heap write #994

Closed
3 tasks done
gsingh93 opened this issue Feb 22, 2018 · 6 comments
Closed
3 tasks done

Out of bounds heap write #994

gsingh93 opened this issue Feb 22, 2018 · 6 comments

Comments

@gsingh93
Copy link

Thanks for reporting your issue. Please make sure these boxes are checked before submitting your issue - thank you!

Detailed guidelines: http://gpac.io/2013/07/16/how-to-file-a-bug-properly/


There is an out of bounds heap write in av_ext.c: https://github.com/gpac/gpac/blob/master/src/isomedia/avc_ext.c#L2415

op->layer_count is read from user input, and then used in the condition of the for loop. This means the user can force the loop to execute up to 256 times. The layers_info array only has 64 elements, and this array is allocated on the heap, so I can craft a file that causes this file to write out of the bounds of the array onto the heap. For example, an attacker could overwrite the top chunk of the glibc heap, which can be used with other bugs to achieve remote code execution in services processing user supplied media files.

@rbouqueau
Copy link
Member

@jeanlf or anyone who knows: a reason MAX_LHEVC_LAYERS is set to 64?

@aureliendavid
Copy link
Member

I don't know (maybe a spec somewhere?) but it looks like it used to be 256: 3d17f95#diff-e61e7a94ff33b9b8d6420afc5577f1e1

@gsingh93
Copy link
Author

Regardless of what the value should be, there's no harm in adding a i < sizeof(op->layers_info) check into the loop.

@jeanlf
Copy link
Member

jeanlf commented Feb 26, 2018

nuh_layer_id is 6 bits, hence the 64. I agree a cross check should be added in the parser

@aureliendavid
Copy link
Member

should now be ok - thanks for the report, reopen if needed

@carnil
Copy link

carnil commented Mar 7, 2018

This issue was assigned CVE-2018-1000100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants