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.

Sign up for GitHub

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

Jump to bottom

Possible problems with sc_decompress_zlib_alloc() #1905

Closed
frankmorgner opened this issue Jan 17, 2020 · 4 comments · Fixed by #1907
Closed

Possible problems with sc_decompress_zlib_alloc() #1905

frankmorgner opened this issue Jan 17, 2020 · 4 comments · Fixed by #1907

Comments

@frankmorgner
Copy link
Member

Problem Description

I think we need to have a closer look at sc_decompress_zlib_alloc() It looks like there's are multiple problems with:

  • calling realloc with size 0 (possible double free)
  • missing length checks when calculating the buffer sizes
  • possible misconception of realloc (afaik, "shrinking down" a buffer is not possible)
@frankmorgner frankmorgner mentioned this issue Jan 17, 2020
Merged
5 tasks
@Jakuje
Copy link
Member

Jakuje commented Jan 17, 2020

I think I was fixing some of the issues with the decompress in #1830. If we are going to change something there, it should be already covered with unit tests to make sure it works.

@frankmorgner
Copy link
Member Author

There are unit tests, yes, but I didn't notice any changes to src\libopensc\compression.c...

@Jakuje
Copy link
Member

Jakuje commented Jan 17, 2020

They were already merged in 07d3d8e. But I agree there might be more issues that I missed

@frankmorgner
Copy link
Member Author

I see, but that doesn't fix the points above. I hope to find time later for some fixes...

frankmorgner added a commit to frankmorgner/OpenSC that referenced this issue Jan 20, 2020
@frankmorgner
sc_decompress_zlib_alloc: check inputs, avoid int underflow
b7690a4 
- turns out, you can shrink a buffer with realloc on some implementations
- realloc is never called with 0 (which would free the data)
- length checking is done in zlib, we just do the allocation

closes OpenSC#1905
@frankmorgner frankmorgner mentioned this issue Jan 20, 2020
Merged
5 tasks
frankmorgner added a commit that referenced this issue Jan 30, 2020
@frankmorgner
sc_decompress_zlib_alloc: check inputs, avoid int underflow (#1907)
a501c0d 
- turns out, you can shrink a buffer with realloc on some implementations
- realloc is never called with 0 (which would free the data)
- length checking is done in zlib, we just do the allocation

closes #1905
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sc_decompress_zlib_alloc: check inputs, avoid int underflow frankmorgner/OpenSC
2 participants
@frankmorgner @Jakuje