dwarf-reader: handle symtab.section_header.sh_entsize == 0

Message ID 20200124225623.59279-1-maennich@google.com
State Committed
Headers
Series dwarf-reader: handle symtab.section_header.sh_entsize == 0 |

Commit Message

Aleksei Vetrov via Libabigail Jan. 1, 2020, midnight UTC
  A broken elf file with a sh_entsize of 0 makes the dwarf reader crash
due to a division by zero. Fix this by validating the input and exiting
early in that case.

	* src/abg-dwarf-reader.cc (load_symbol_maps_from_symtab_section):
	Handle elf file with invalid sh_entsize.
	* tests/test-read-dwarf.cc (test_task::perform): handle empty
	in_abi_path and out_abi_path as 'read only' test.
	(InOutSpec): add test case.
	* tests/data/test-read-dwarf/test25-bogus-binary.elf: new test data.

Signed-off-by: Matthias Maennich <maennich@google.com>
---
 src/abg-dwarf-reader.cc                         |   5 +++++
 .../test-read-dwarf/test25-bogus-binary.elf     | Bin 0 -> 95769 bytes
 tests/test-read-dwarf.cc                        |  12 ++++++++++++
 3 files changed, 17 insertions(+)
 create mode 100644 tests/data/test-read-dwarf/test25-bogus-binary.elf
  

Comments

Dodji Seketeli Jan. 1, 2020, midnight UTC | #1
Hello Matthias,

Matthias Maennich <maennich@google.com> a ?crit:

> A broken elf file with a sh_entsize of 0 makes the dwarf reader crash
> due to a division by zero. Fix this by validating the input and exiting
> early in that case.
>
> 	* src/abg-dwarf-reader.cc (load_symbol_maps_from_symtab_section):
> 	Handle elf file with invalid sh_entsize.
> 	* tests/test-read-dwarf.cc (test_task::perform): handle empty
> 	in_abi_path and out_abi_path as 'read only' test.
> 	(InOutSpec): add test case.
> 	* tests/data/test-read-dwarf/test25-bogus-binary.elf: new test data.

This is OK to commit to master.

Thanks!
  
Aleksei Vetrov via Libabigail Jan. 1, 2020, midnight UTC | #2
On Mon, Jan 27, 2020 at 04:20:21PM +0100, Dodji Seketeli wrote:
>Hello Matthias,
>
>Matthias Maennich <maennich@google.com> a ?crit:
>
>> A broken elf file with a sh_entsize of 0 makes the dwarf reader crash
>> due to a division by zero. Fix this by validating the input and exiting
>> early in that case.
>>
>> 	* src/abg-dwarf-reader.cc (load_symbol_maps_from_symtab_section):
>> 	Handle elf file with invalid sh_entsize.
>> 	* tests/test-read-dwarf.cc (test_task::perform): handle empty
>> 	in_abi_path and out_abi_path as 'read only' test.
>> 	(InOutSpec): add test case.
>> 	* tests/data/test-read-dwarf/test25-bogus-binary.elf: new test data.
>
>This is OK to commit to master.

Done. Thanks!

Cheers,
Matthias

>
>Thanks!
>
>-- 
>		Dodji
  

Patch

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index d3efb02a9dbf..555170eccdf0 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -7388,6 +7388,11 @@  public:
     GElf_Shdr header_mem;
     GElf_Shdr* symtab_sheader = gelf_getshdr(symtab_section,
 					     &header_mem);
+
+    // check for bogus section header
+    if (symtab_sheader->sh_entsize == 0)
+      return false;
+
     size_t nb_syms = symtab_sheader->sh_size / symtab_sheader->sh_entsize;
 
     Elf_Data* symtab = elf_getdata(symtab_section, 0);
diff --git a/tests/data/test-read-dwarf/test25-bogus-binary.elf b/tests/data/test-read-dwarf/test25-bogus-binary.elf
new file mode 100644
index 0000000000000000000000000000000000000000..b00a9696232bd813aa413808e56e6875a3069a03
GIT binary patch
literal 95769
zcmeHQU1(j$5#DpH9645MnnF~mgjgoA<3w>}OLlA*Rgw`>lb}Kirj*DgT+4#UBzllj
z#iDSp?G)u9bx0mm`jog(XbB`xLPK09G=A`VLfz5^0wsk8QCvd|j-9l3`_1g0-+S-T
z{nP!^4AMF0o}c}hot>SXnVro}OiiXzPU^($@wxQ;?&EXwOOx4!^wh*eHl;RYl>x_|
z)j6eb?YQn8{YrJ<?!*pyG#z}NURctAhjMh(`t7&h{HIc#i`j{s%1%uEY%=9Il*H=w
zYpY?n&;ICse)c}CQc4Y*@AWlvrd(c~rZ3elrBcJgdRQa9Nyl+cGfZinH8-@$kDpNy
zJJkWDev6OPT`F_q!r50cnascd9%ROQI6++Eq08@X)kn*Qu|)Xn?1jYVV`GOy&r#d4
zv98dKv9Y6Pw5#1;-#Dt>qbQxmzk30<0O~uy)Mll|Q_gc^3p3AbqpB#aAHe^sIci$3
z<y@80t_6O{9#&WB0#$d^l!3?XnNEaWgTnpX@SC;|@0N_33w5F9YW$@~VmQx(6S$9r
zZqTLevY@HEnKFNGyCu4$>IPn^ORAL>COq`ler3MDbsJFp$sFeu%BpbQ|9byLMJyvJ
zlM<6MoTo9>h~*8eF=tyFg%@wvx^|T{aEMGdIbCYh0Tc5%v89kR;BH;i(azkV)EuhV
z2j=Zpv1c4lvg19Vp%j=G-CtbHEH3KLAGV;nzyk5W<A0Qt>LpHB=;h!kU42`r8yC*J
ziIIwUAD05>^`grB<}BC?Zh^e17gXDX53PSct8<O&z&To>MxpMpo=mhAP~o~fU{uys
zhyxTFA!^kDg<Qjfy0noIgoNsvP+)9sURRjQPt`_NZtki_<^_#oA;idJSn#w2RnF*|
zn4HyDuf2YL`SzbfGfcWHYckv0+ethgu;Rf$Zye+m!j0gC=|YG>h{0zNLJUF-LJTr|
z%GfAlBXL3oAqF7^j}}B~Hp8O`N<<M&&lxa%hDU*OmD~^9T}O7}bW@YDj@%j;g4s1Z
zIIQ~to*P>yE(ly}HVzr*cDkw`iKl3SP7WWL3{u++hYyopCwY8yz$4@ixQS3&fT$mc
z-eC(6J@Mzhy9j-@0fCT^;)COiu+Pt>dE1CEl{#wEEU<b?sXGmB@ME+irH+vqm9Y6S
z{_WS?@a25tL|lx>h&dU(HLALIA4djwKzT8M#u(=5<4g}ZIB`41yjt*1#DpFqClFDX
zoRmrz1e>aFwf@kzT5rx|&YjiQkwqroYW+bJtPy+h^M|*5!gyl$ooT+zexJN~yo%BJ
zfB65Esh>jE=>v{I+uVulUH|k+49MjR50UZ@(4Jn(D)!bVg<tc_fC5~}2R-DyP`pf;
z%>DhQ>pl%TzR@)c?yg*E3EX`kbQkKj(r1aw2){$85Pp|8q<BLC4OaO>MQiXo8D~T6
z?t?bvcfM}5x3{P~V#TSy;2f6a&J}hlgd^e~Ja0stA3QY^`la=C;ulJNsnt#O#1l`x
z%K%46Wv)U;+lQ<}z2BoE7s`{XNepys4M^<nRD;HI8`lk<I^FV1Om5{UO#W2OpQgd{
zavBQ-P<&<QYFqu;qF)o1AZecG>UZO_Hg(UJc4CLwCBER<B@5CtFEnY{@&x6g$%CUl
z@+Jd?CWn3<D!#xvgHtn9vmq*Zdi!u1j{76Uj$1-b)XUa!D!;aC)g<#N<HO8!iz7s)
z;zTK}0Zhw$kg0fNN^Y5-Oeb4pIw|5+<>Hl0#Vg01(A@^b-PixwrA7VN^3!oz!7F<2
zcAbys+cCsufDub{Xx0iQwkpyd#i29<p5RV!C-!BmC>Ps&0e=A*XJwofpOE6VTpDv_
zoE2ZBaP*O|CI#1RY9p07neQs+TmNLNsCuj*>rHH4+Y)|obx|NkOj|Yh0o%3-bZ5=a
zVe>NCqhU)o5DS44_JcL=`A2JTGu{Sptft9T%(FDFyI8}e>Vv^O$3p0kQjbuuC5jJ-
zyGp%L8=$06X^#8Uer%d+CkMfRU_dai&KQXB`Gf3L7FjhBT}I=_ZgkzPG*oyY8F?u1
z#}2xPq`NmU-C~KXk;6J8Mv@l{2nGZLb;3Y|Um~<M<n7w?;laK84(vNP^w_>bi5avo
zL(W?$CyH-TW>l@rXlg~qH+b4zP>CfgNqyyjXm4pClNEiKsT8*Y$zUOaMSBkxGFC)Z
z`L(SIS(b@dYraIgz_2f#4dU6*cAgDlVNURxu7OcGl7v{8{h4GtI0Ds4sVUhA1_T2e
z90L){hFF+4I5kN11p|VCMqnVqhsDC&h$fIc1p{@)z}kvngh%r{P4YHSr@}UaQLI$|
zcHbr4Uc{roaZUY=sNojh4xyG6?E!!PPV%}082TEz|0l|iag7LZ<{6Ic+rz)KKDIV!
z)j_WODyLu6e|7WX?dhwEx0*MZmVUHj5l4ykLPg6x$BXgUPI#${*pcjDOugRQT(UgJ
z<=uk^4i4@+v}b5vqQhL}8PuxalhTW(1P6$@FLWR{AeJk1AUF^@aN0?dl0GebP3Rdb
z$%U^84zSsP(1GB9Hk}YU5F7{{nEm#oFGydIz94;}9W4nm|CRZ#jBn+RZ>1Y$<xRnW
zU_dY+7!V9pgn{i8Boy#N+6eQ|;NZ~Efx$gPj~(2<@4$im2lp7C(@l86^EXr7_>m5R
zlJXPM?ozx^HMd#-W>;^-oV?(mW_F#=WP9k-l#@C!dwecEzx((cyq#8%#W-BP?yV;v
zPryGe{v1a~ZR(?7sJ+=AVL#UZMf3&(Dolc%fj)ayEa{*!^!u%>xyz`*Ev*kz4cRA!
zH?4Ku<>h7nOSupX2nMRbKvQ!vQ!!yK)ikhtD;TI52H;LjBQs1<UbytN!Z+3|Ye`5j
zAQ%t~2nGZLf&syRU_dY+7!V8y1_T4mhJhRu_sv$OjZhxXpwb6>K}1inpA86sLB=A(
zHRcqhR{WLnJ6(S<xvTCXiwoLvb7o+`q}NIMjt+%w;%qPhq!isaNbiVQZOl*pTsgrq
zeNv*pI+UNEpBHp2(_|g$L>?x?t;%t((lT^f`yMhanN{xZ#<N!Ud}&MP?mB`m@)ort
zEnC>2TtvB!`p8TEF8VH3!-5qo>n8T_J2gW!8=|78w-2WmW}X?@65h>Eaiv_HSF5#3
zocWaNZ3<ny;y~iABRefpV72l<=tc|OIOWrgv0{Q~)kD-}07OwRiK06>S5?<&%IO+i
zMYqCFemA5<C1&DxZK8W`UmDW~>@lFsyV+Ly`v?y(y)?whyf^^2ZUcJ4JTCHtEXPaX
zeAddFcn?8{|0L&Kb`FiK9p~MKwF_Jic-X0q1X`$pF5XTgzu$S3-z1%@jmU3rG1RtU
z?_ztH;mT4wv^$SEi5<@)j*vaEp`%(GbvzHXah6sZWft7dnl_u$PCp4V0hpW7drDcK
zwm$lR|M3m)jExURl~6{p)@hJ5S03F5jW1W#VSqYkiVu!6!d{%0=I!Jjt`(Rz%wkS(
zN~t?dHuy2xky6LV?SkeNALHMC%?g2XzH!1%qf6wOIvKq+s=9X{M+SI6c`<lW|2%!1
z>G6C{m-IN5s4F{8Z}o%a_xyNi<hu`C9y#WWs4Pzm_8vI&wfYmm;W37z`~wG0S?k}}
z$w^AtSKshoUAEmt!Rs@poHO{Mi!kyb{^u|U>0=d5hTUt)KXkLoqCljx(f#;8TP;vR
zd0_I+t(v^`_0S880PiPWd-46xaipn|Cvf{w*_IYuu*Krf5VDcL)}ODr=F}`t<2&35
z<)uePMpPg3TeSs@w<DL!>0AzXSPv4KaJ~ht3;QRJ_XG`NT9@)7M3b+w`X>E=q6rg+
zq5TJK19E8;x!~rgrj-DddQlF7gGSN;6W|6W!jnl*e#3a<*i*A}%&6>v*$?p&^ZUQy
zXShC*o*#%odf}zlUO&Hl`N}vI;xc$z*0hrO6bXUP;pRlnsN0}Qg8;L-dac0viPhEa
z=V$NJq;VUzIbY**uwr5<JCU7Gso`Ng%%dq(a;Hz9rklJlBTHo;cy9jrfm1UJ>7UME
z$fYNaqoKOCY@T0@EyUntgA}NUH!ht0C8b7i4*#3UiU47GfNo4vv7V#>U8*ImRhS43
z;T3he1={jPdJOb?NR%Y;;~1|US~>a7#hj6q#={!nKoE#XXq2W0qxQnCKq`{o;C?^c
zesYex4-FQQ3BBCt?B$5d!{Ph5jta1)ykHWz<hp-j>1Emdf<+k?cXA=-(XK4x(E;Iv
z%(}~F@SbflLUYj}q8%AL4M{R?)kMmOF)#Tk7-$RzJi~Nja+h4Iz(5wfnBZG9h$(e9
z3{Lh=q8&aWHO8Hw@|~Zc@#%+Llp@*4I6uJ|3Asl7f8e|mxbB2qc8o|xBL^U6a1JMJ
z-t&)C$el572+z47KYN~!DeFLFTz$BBi5?q~yi8U#vo`Sz5K7#qW-*rx0gK3?-;v7#
z4P(%zolPNSU5T$j(vLRou8e7;_+2zPzuOJsAzO|S<1dBh0=5{>G7&J|xPn-VPxR8M
z26;-sg)}v?xDARbc2Hm2+wyi$BLJa^+uf_F6mIJ2@LRrBt-U0o187N_jSVszrh&w(
ziAJ6Hkkb+-bU0(KlVi-f^=fiex47o<z!OkveD4A1>ER~}p~NT|9o2_FW7C8#g5;G7
zfrKHz;qt+JSnxPq@NVoT=}_u{JINzXqt1lmOlGHQOrwJ5IG!zn1<!Tik@clN?O<xY
zPTZbr5)WY)x191^)3DTYbYh$74!UBSDdwt0Y^K{nHXXh!V%UKtSE*O0NrSI;VLl8~
zuz%s%gy|Kf{vAotPYwt|2)``Zm`xWb?d>(~$|dggfK?-u?RvHmI{bQ$`t_w~!IiY7
z&@&12LI;8ap#zKut=Shmr#h>yFaOkx48X`v)g7x>0)?$1wa|*9KAt(NfbUf;a<Fy2
zX`Qo-%XbjdAfgs++1jCUYiqc;cF$6u4*A?yp|7u7+m&8B=2!`}qvhU}X&76uvc~wh
z){^y&Y1}cmsolbV#$xsCHA0{BX_PYLLtx5mj|H2xy3s+t77Wx31Nv6Yl9Gg47z2h~
zv6Qg^q^q^$4}*<@yxSTHpv5i8N2rBKLW<tD7{J`ZC*Zg=RQFY#N$dDd)@UK0(G-wB
zAj~l~{+xtPBQ}>GoK8ZgN$4~)0Q$$!pl6oZ*GnB#JC%Z|OSMC~j|iSf5*Nh<+=@AN
zL6PACi+!Q=@o{M`AY?(sWxseYgYXq<bLU}n$1ZrS*IwBrPKG}I!z2(1oyOa@y#iY}
zmO`r&T92|q`q2Rsr6g&{Ov0{6(-!k#yt@)uZGF|~&Bixtq{@!Vs?zc5Yb21sYQ)TZ
zA)quBKx8zp%&O56bYDdLDetx>pN{W2PB#y}?^v7gUVF|zQCncV&VN969Pc)QX*nE=
zA*H6by&PTRR$lqq=xs)w!JmY0lkja;OxsH5`80)glkjcOK5gI#TjlHC<cpH&MZ&iw
z=zYtlBX1m&@NLIg#BBfQJ|u}j`6f~6c=ZOMqQn@2SzTESLF!^g3+9Dijrar>38yCE
z+psrDFl&(TZ4$^*;&bx6oQLd$u%kSK7Y1)7n=%-@&SUTa|EcKe-ip3zvyY}!C#|&X
zpU_iGzF4tX0D5CU8{ZMEcrXxqp|`h+SEho*sz@bBq?)owD(l(Of?dJx%1TX{hme$z
zl#rC|1si4fj7;M9vIZr#u+Nh>`|vQq-W!c&L@|Q$;~TMU&QVB-p9eoHO&D7aFa#+D
z83!A5l^%pCf(<|HJI9$P|BSdw<L~4(+h_Nl@s%GYB7G^kkzc?W2w6sQjfS_R;Fb)s
zt+U;6L(8_#o)w$J9k36x7X{f&kr7p_y|(8CGhv8vrdqqwRpnJ-SP+8d08n00Lzk<D
z&eu|GxUFo$L#+%7vS+)hVBvw#jnGXcbR)t9bh0&E2U?XdK|WViM)*J^s6I#{!VZKT
zRKgAn-5hzL|40DXruAi|w$UTn=2yRf?P#$r>lfa91n2ik0rP(B!^iPuD4|j=zrsro
z7Jt!hEGHA{BQz(uiKgO~18d>=;cE30>iF)L#oHqE#BOuasfLKlrM<n#LIG2j6+J1m
z6xAkER0WVRo2Qt~T@rUsDY)1-Ku{wl)FB+%OLPfpx(K`(2in<K*1gns-3tciNTbLP
z!9X$w&_fMVlh{?sh?1v*feJAov8yUXT$_Dn7;Ov1u4*&<OLZEI0bzs<Zh$7qzU3HU
ziv(3^lIE3S2nIqJNa{RwU$>mFnDb<<g7|5j>$CMY3bVLA@<e)mAO`7$mtK4Q{PN{1
z<FuyqGGcLM&5CZ50(7Dy04M@;Mi~e*3cNW=TuIlyE({a`Af%x+14{!VpUAcVx7qvY
zS=knV3DYA;oSTbKCGeIZ)EZjW!SdsdfTsCHwHlHJ<WewDI}C_Mp?0`z<&+GgU2oOu
z+Iu3E9yYI)u`Ok9J`4yaY`!*;5(x$b1A+m;Koeo$voq(Ls5A*hi4^qOAKyxNwc(E<
TWo>wA5(>~V1x=_!vpxDhWLwe2

literal 0
HcmV?d00001

diff --git a/tests/test-read-dwarf.cc b/tests/test-read-dwarf.cc
index 7c656964c93c..8f460fef3f8f 100644
--- a/tests/test-read-dwarf.cc
+++ b/tests/test-read-dwarf.cc
@@ -253,6 +253,12 @@  InOutSpec in_out_specs[] =
     "output/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi",
   },
 #endif
+  {
+    "data/test-read-dwarf/test25-bogus-binary.elf",
+    "",
+    "",
+    "",
+  },
   // This should be the last entry.
   {NULL, NULL, NULL, NULL}
 };
@@ -324,6 +330,12 @@  struct test_task : public abigail::workers::task
       set_suppressions(*ctxt, in_suppr_spec_path);
 
     abigail::corpus_sptr corp = read_corpus_from_elf(*ctxt, status);
+    // if there is no output and no input, assume that we do not care about the
+    // actual read result, just that it succeeded.
+    if (in_abi_path.empty() && out_abi_path.empty()) {
+	// Phew! we made it here and we did not crash! yay!
+	return;
+    }
     if (!corp)
       {
 	error_message = string("failed to read ") + in_elf_path  + "\n";