Bug 706087
| Summary: | libvncserver-0.9.7 unallocated memory access at tightBeforeBufSize and tightAfterBufSize | ||
|---|---|---|---|
| Product: | [Other] Security Response | Reporter: | Petr Pisar <ppisar> |
| Component: | vulnerability | Assignee: | Red Hat Product Security <security-response-team> |
| Status: | CLOSED WONTFIX | QA Contact: | |
| Severity: | low | Docs Contact: | |
| Priority: | low | ||
| Version: | unspecified | CC: | jrusnack |
| Target Milestone: | --- | Keywords: | Security |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2015-09-03 06:41:01 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 1247623 | ||
Analysis: It seems this issue exists because libvncserver < 0.9.8 is not thread-safe. "tightAfterBuf" etc are declared as a global variables. rfbTightCleanup() which is responsible for freeing these variables, do not reset them to NULL. Consequently when other parts of the code use these variables. (For example, realloc() after checking if they are NULL etc). It can cause memory corruption. This is specially possible, when multiple connections are made using libvncserver library. This issue has been fixed via: https://github.com/LibVNC/libvncserver/commit/804335f9d296440bb708ca844f5d89b58b50b0c6 As mentioned in comment #0, this breaks ABI. CVE Request: http://openwall.com/lists/oss-security/2015/09/03/8 |
libvncserver/tight.c:rfbTightCleanup() frees a buffer without zeroing freed pointer: void rfbTightCleanup(rfbScreenInfoPtr screen) { if(tightBeforeBufSize) { free(tightBeforeBuf); tightBeforeBufSize=0; } if(tightAfterBufSize) { free(tightAfterBuf); tightAfterBufSize=0; } } The tightBeforeBufSize is global variable used on other places in similar manner: if (tightBeforeBufSize < 4) { tightBeforeBufSize = 4; if (tightBeforeBuf == NULL) tightBeforeBuf = (char *)malloc(tightBeforeBufSize); else tightBeforeBuf = (char *)realloc(tightBeforeBuf, tightBeforeBufSize); } which can try to reallocate unallocated/other memory region. This problem is explained in upstream commit: commit c1363fa9583ed41b94fbc79b3ff410b7d5189407 Author: George Kiagiadakis <kiagiadakis.george> Date: Wed Nov 10 18:57:23 2010 +0000 Fix memory corruption bug. This bug occured when a second telepathy tubes client was connected after the first one had disconnected and the channel (thus, the screen too) had been destroyed. Signed-off-by: Johannes Schindelin <johannes.schindelin> The commit itself is useless as the bug has been fixed already in: commit 804335f9d296440bb708ca844f5d89b58b50b0c6 Author: runge <runge> Date: Thu May 21 10:32:18 2009 -0400 Thread safety for zrle, zlib, tight. Proposed tight security type fix for debian bug 517422. The huge commit fixes a lot of other bugs by utilizing thread local storage. Unfortunately it adds new members to public data structure which breaks ABI, thus I do not recommend back-porting it. However the small fix for nullifying freed pointers is good candidate to apply to current libvncserver packages: void rfbTightCleanup(rfbScreenInfoPtr screen) { if(tightBeforeBufSize) { free(tightBeforeBuf); tightBeforeBufSize=0; + tightBeforeBuf = NULL; } if(tightAfterBufSize) { free(tightAfterBuf); tightAfterBufSize=0; + tightAfterBuf = NULL; } }