From 5519497ed3de7a1c537872bbada9d05260cbfa24 Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Tue, 30 Oct 2018 22:56:53 +0100 Subject: [PATCH] Fix for #85 and #86 Is longjmp() better than abort()? --- lib/component.c | 4 ++- lib/helper.c | 10 +++++-- lib/internal.h | 27 ++++++++++++++++- lib/libunshield.c | 6 ++++ test/bugs/debian-776238/data1.cab | Bin 0 -> 655 bytes test/bugs/debian-776238/data1.hdr | Bin 0 -> 3036 bytes test/bugs/debian-776238/debian-776238.sh | 35 +++++++++++++++++++++++ test/bugs/debian-776239/data1.cab | Bin 0 -> 655 bytes test/bugs/debian-776239/data1.hdr | Bin 0 -> 3036 bytes test/bugs/debian-776239/debian-776239.sh | 35 +++++++++++++++++++++++ 10 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 test/bugs/debian-776238/data1.cab create mode 100644 test/bugs/debian-776238/data1.hdr create mode 100755 test/bugs/debian-776238/debian-776238.sh create mode 100644 test/bugs/debian-776239/data1.cab create mode 100644 test/bugs/debian-776239/data1.hdr create mode 100755 test/bugs/debian-776239/debian-776239.sh diff --git a/lib/component.c b/lib/component.c index ee07d013..89d2bd21 100644 --- a/lib/component.c +++ b/lib/component.c @@ -51,7 +51,9 @@ UnshieldComponent* unshield_component_new(Header* header, uint32_t offset) self->file_group_count = READ_UINT16(p); p += 2; if (self->file_group_count > MAX_FILE_GROUP_COUNT) - abort(); + { + UNSHIELD_THROW_EXCEPTION(&header->exception, "file_group_count > MAX_FILE_GROUP_COUNT"); + } self->file_group_names = NEW(const char*, self->file_group_count); diff --git a/lib/helper.c b/lib/helper.c index 6de22b96..41312192 100644 --- a/lib/helper.c +++ b/lib/helper.c @@ -174,11 +174,17 @@ bool unshield_read_common_header(uint8_t** buffer, CommonHeader* common) */ uint8_t* unshield_header_get_buffer(Header* header, uint32_t offset) { - if (offset) - return + if (offset){ + if (header->common.cab_descriptor_offset + offset > header->size) + { + UNSHIELD_THROW_EXCEPTION(&header->exception, "cab_descriptor_offset + offset > header->size"); + } + + return header->data + header->common.cab_descriptor_offset + offset; + } else return NULL; } diff --git a/lib/internal.h b/lib/internal.h index 8120475e..ddf1fd20 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -11,11 +11,22 @@ #include #endif +#include #include #include /* for FILE */ #include "cabfile.h" +typedef struct _Exception Exception; + +struct _Exception +{ + jmp_buf environment; + const char* function; + int line; + const char* message; +}; + typedef struct _StringBuffer StringBuffer; struct _StringBuffer @@ -28,6 +39,7 @@ typedef struct _Header Header; struct _Header { + Exception exception; Header* next; int index; uint8_t* data; @@ -63,7 +75,7 @@ UnshieldComponent* unshield_component_new(Header* header, uint32_t offset); void unshield_component_destroy(UnshieldComponent* self); -/* +/* Internal file group functions */ @@ -84,6 +96,19 @@ const char* unshield_header_get_string(Header* header, uint32_t offset); uint8_t* unshield_header_get_buffer(Header* header, uint32_t offset); +#define UNSHIELD_HANDLE_EXCEPTION(e) (setjmp(e.environment) != 0) + +static inline void _unshield_throw_exception(Exception* exception, const char* function, int line, const char* message) +{ + exception->function = function; + exception->line = line; + exception->message = message; + longjmp(exception->environment, 1); +} + +#define UNSHIELD_THROW_EXCEPTION(exception, message) { _unshield_throw_exception(exception, __FUNCTION__, __LINE__, message); } + + /* Constants */ diff --git a/lib/libunshield.c b/lib/libunshield.c index f9531162..1f2a7b82 100644 --- a/lib/libunshield.c +++ b/lib/libunshield.c @@ -240,6 +240,12 @@ static bool unshield_read_headers(Unshield* unshield, int version)/*{{{*/ Header* header = NEW1(Header); header->index = i; + if (UNSHIELD_HANDLE_EXCEPTION(header->exception)) { + unshield_error("An exception occurred while reading .hdr file %i: [%s:%i] %s", + i, header->exception.function, header->exception.line, header->exception.message); + goto error; + } + header->size = FSIZE(file); if (header->size < 4) { diff --git a/test/bugs/debian-776238/data1.cab b/test/bugs/debian-776238/data1.cab new file mode 100644 index 0000000000000000000000000000000000000000..57dd88cd18f302ce6392b641087de3c56a1c77e5 GIT binary patch literal 655 zcmeYaPS#)yVqjzd0VXhspn+T#sL&`yK?pqf6gNlM;;P`*84L|4%2RmIjXGg}qTwEr zJjdPenQLwV)lY+{=S1V9>qqAoEc3JZc;vF5b??(GKFLFB=@t9FUW?A(cvP(Q(2H$% z%cA4n?tDJ4?ZI=k*}V*+y@q^;36z5ubprniUG>v%EtvNtkA>T#{N9Db4%}Ck zeRw9uG9ni4@U=b}hn@n`clHDxwOn;VOFVCf9* zc?-@^2augiv=&$+y%Q*dHbQR#_;fM0LG#HgQpF-CZ!=Hv|vQd0n)+bZyC(V5^Zz>BpU0`j2~WsSwq1)}ZHZ~0=F7kX5*ANrZ- z{m_2YKLHLxKa}`M=uGKz9y(WaJM^&BT!$`|_$}y_&O_HOfqOprRaT z1}*@1f&Xm@^gKF?*HGVprJi-QrjYfq%;USUd zHR#29sgJQesw&7%$W2tEtECx`Vzzu(^?UJ?1vV-omd)uKZj#9a9JyBY-k_rE!lD{8p+VIPOpDi7b hXC|t)LqrUfk3>&jIVzP&(Jzz^5$<{FnxvS}^Di7k{`~*| literal 0 HcmV?d00001 diff --git a/test/bugs/debian-776238/debian-776238.sh b/test/bugs/debian-776238/debian-776238.sh new file mode 100755 index 00000000..8fb3b4e0 --- /dev/null +++ b/test/bugs/debian-776238/debian-776238.sh @@ -0,0 +1,35 @@ +#!/bin/bash +set -e +cd `dirname $0` +MD5_FILE=`pwd`/`basename $0 .sh`.md5 +CAB_FILE=`pwd`/data1.cab +UNSHIELD=${UNSHIELD:-/var/tmp/unshield/bin/unshield} + +if [ \! -x ${UNSHIELD} ]; then + echo "unshield executable not found at $UNSHIELD" >&2 + exit 1 +fi + +DIR=`mktemp -d` +#trap 'rm -rf ${DIR}' TERM INT EXIT +cd ${DIR} + +set +e +rm -f /tmp/moo + +timeout 10 ${UNSHIELD} -d extract1 x "$CAB_FILE" > log1 2>&1 +CODE=$? +if [ -e /tmp/moo ]; then + cat log1 >&2 + echo "unshield vulnerable to CVE-2015-1386" >&2 + echo "See https://github.com/twogood/unshield/issues/42" >&2 + exit 2 +fi + +if [ ${CODE} -ne 1 ]; then + cat log1 >&2 + echo "unshield should have failed with error 1 but was $CODE" >&2 + exit 3 +fi + +exit 0 diff --git a/test/bugs/debian-776239/data1.cab b/test/bugs/debian-776239/data1.cab new file mode 100644 index 0000000000000000000000000000000000000000..57dd88cd18f302ce6392b641087de3c56a1c77e5 GIT binary patch literal 655 zcmeYaPS#)yVqjzd0VXhspn+T#sL&`yK?pqf6gNlM;;P`*84L|4%2RmIjXGg}qTwEr zJjdPenQLwV)lY+{=S1V9>qqAoEc3JZc;vF5b??(GKFLFB=@t9FUW?A(cvP(Q(2H$% z%cA4n?tDJ4?ZI=k*}V*+y@q^;36z5ubprniUG>v%EtvNtkA>T#{N9Db4%}Ck zeRw9}Y)+&DGzyANR0LWX zL{v8&E)1fJNP~zhjfhgANV|xHqyme)%bs`tyYX*g-iY1!j%UyF9{%rpzwiI{&Eodw zXk4TrSU8DjOiPrzki9vZu^sp{R&_ONd;j7GmnhuRS~F>Tdy0C(L#n*?qxbUN_o#1E z>eCba8EcGa0PmaaxV(GTzXXv(0i6QO6V_03G9=8oD4JLN+5ApVo6Xtg=HeY#I)i)O ziZj#!1=h&eNt8t!p*I11x)|G_`2?1zV$qYg88Nlx!QTpO0rC@a>N*WAkyEUW zD%ybOoIp3+b3h8sSFT4X<+-bnuLk_UUcd+J0j4tUUxUwX)TE|I9HJv=;TTYZU_H

x{>o8VibJHhuwcY(V__kmxD9sm!D9s&=G{sJBm{T=*6 z^l$LE=s(~I(W$sn(*W*m6?nDiY;X?X#aa#lh0w{e#!~1a(RS#!{4mT5Ju2D{{ao~Z zXg}(o0tcZVN&F;qwv2NgI$v}<^sv<2fG(H#ZRnNI3$VAH(Bo3m1)V81N+&x|Q3x2JJa{URpLHNt$Quqwbe~4_(U|?U+*Pt`k z)z>yFAtHT6v8&WnVYIjkQ4y#O1rJC;oFnCiKtq$yADT3n-sy6bx{7UPi`C@DKzf72 zW^>qMuGeB$X3#^qE8XI#=hnMDPN%~qo#@>zgA|pOTWvgKS-HU@Y*5%}RM-=l5_w*O z-d$$0TB{u;J57d)YO}*?G)2N5J7%t!vLH4g=HcbX;N2;v6S zE9MNEVZu`4R*V=g)8?yd#O}I7zF?@OiFZBjHN?NduSl-|N`QKx4R`{C0X8k|+JGaT zL8OCMkV|9;E4MeUR4w6HoJE`sr{pM6Z=O=QZaq;(i;{_aqmutW*tC($fPY&$pukL2 fYlny$Dj$oUzH(G5Q)2&6Iz)KPQ`aTMgfagD7{vbl literal 0 HcmV?d00001 diff --git a/test/bugs/debian-776239/debian-776239.sh b/test/bugs/debian-776239/debian-776239.sh new file mode 100755 index 00000000..8fb3b4e0 --- /dev/null +++ b/test/bugs/debian-776239/debian-776239.sh @@ -0,0 +1,35 @@ +#!/bin/bash +set -e +cd `dirname $0` +MD5_FILE=`pwd`/`basename $0 .sh`.md5 +CAB_FILE=`pwd`/data1.cab +UNSHIELD=${UNSHIELD:-/var/tmp/unshield/bin/unshield} + +if [ \! -x ${UNSHIELD} ]; then + echo "unshield executable not found at $UNSHIELD" >&2 + exit 1 +fi + +DIR=`mktemp -d` +#trap 'rm -rf ${DIR}' TERM INT EXIT +cd ${DIR} + +set +e +rm -f /tmp/moo + +timeout 10 ${UNSHIELD} -d extract1 x "$CAB_FILE" > log1 2>&1 +CODE=$? +if [ -e /tmp/moo ]; then + cat log1 >&2 + echo "unshield vulnerable to CVE-2015-1386" >&2 + echo "See https://github.com/twogood/unshield/issues/42" >&2 + exit 2 +fi + +if [ ${CODE} -ne 1 ]; then + cat log1 >&2 + echo "unshield should have failed with error 1 but was $CODE" >&2 + exit 3 +fi + +exit 0