From f0ae945ecc4631c538b845d807a60c5b72903a5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 11 Apr 2019 14:01:38 +0200 Subject: [PATCH 1/3] bus-message: validate signature in gvariant messages We would accept a message with 40k signature and spend a lot of time iterating over the nested arrays. Let's just reject it early, as we do for !gvariant messages. --- src/libsystemd/sd-bus/bus-message.c | 5 ++++- test/fuzz/fuzz-bus-message/oss-fuzz-14016 | Bin 0 -> 49343 bytes 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 test/fuzz/fuzz-bus-message/oss-fuzz-14016 diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 11c4648f91..a2464e1a46 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -5152,7 +5152,7 @@ int bus_message_parse_fields(sd_bus_message *m) { return -EBADMSG; if (*p == 0) { - char *k; + _cleanup_free_ char *k = NULL; size_t l; /* We found the beginning of the signature @@ -5170,6 +5170,9 @@ int bus_message_parse_fields(sd_bus_message *m) { if (!k) return -ENOMEM; + if (!signature_is_valid(k, true)) + return -EBADMSG; + free_and_replace(m->root_container.signature, k); break; } diff --git a/test/fuzz/fuzz-bus-message/oss-fuzz-14016 b/test/fuzz/fuzz-bus-message/oss-fuzz-14016 new file mode 100644 index 0000000000000000000000000000000000000000..c82d1ba4adfbc829ef148bfc61907c51f1fc9b74 GIT binary patch literal 49343 zcmc~vEM{UbPE1TptTRtCWcb3$z%Y4I+9VE!0RNg|YDDnT+J zKR+cOHJb4FhBzsR=`dSCy!`z9q$F&bk>V4ijzH|fjD*nn`S}n%NCra$Ao4I0n;1+E zD^0AyFwI~KARI6gTXaLDladH3fGI?wA-a%B6lr7*LJ^WEQZOTlBA8%J2rhM5=)r*$ zT{xLw2cc^OISZ!*ND?Q8=t3rmP>QJp$$){)HqeBR;zFEZNsI)YB#9G4bRm;OD8*EQWWZ4Dra(MIY!e^iS%@B@ zNMa3!83wih!T~d}MK?qmOwyv=1U48<5aR-{LKraweM?Nn!>NYk00IZpAP*(58$m;` z5Z@tr9wGpdC!a)5(nuEIWa6|LrvzU2L3AOLIF%z5p(w%WQeq?^W}=u!DhF%?9!+3* zBm!SOjJ+bnE{exWs1$NN4G#>0eMz`VLJWwXpmrfs*d!ogU~>4^O|Za#dY4c@5-krl z51wg3JbZ}>#T9T}Fi|8LWYB1%4%@&5B#mj_7*!({PeG3T&GD(sH2pnjGJe0t0BsxbxRFguY*963bJ3z;NBDW(!6 z1BQRbfp``(5O9i+l_Mdh5o6crNHi#vAmN2Xf&}3hDQiK*kQ8EH^opbdKNE|2D29VL zqa$-z!VszKB_<3(;RvfJkW9m(6-kzKCKmIcHWN*OjliQCERRIss~?d93CTq4qDTsG zFh|oNmIQ$mn2<=p5qQ{y$tv2hxPe?DR0o52_!1M$2OxEb#R4!shBEfpfd>*)nqbhvGZ0h*G6f1~ zyvY<09t=rXVl30TEwlNKX*;L(N@!Qik$PXJJzU^xgI%*3G#q9Q4Y zf+j7@2yhG{I9OOiV(U7k-`Zl6YL;#jUd7zfrMltc2Oh+ zIG9NN5wQI@l;IR6D^Wm91KWohPY?+tl7eao9MKS?NOdGc7cnHd^TC!9AaME{rvy%y z;M9Rr0;f?R35YIa5+sThBNSmuBjiRGP+*1*sfiIBnh;~aO!OoNk%E(y<`~?IazIuP z;{uR6I39w&C6ImbOOqTx;E)>Rp#*m0$e!md*i zUt)sA3`iZ`G!9XKWFtfXgM=A~K_gTmNfBxpgEb+krz#U3NKjLeJcpABb`ZKokbayJ zAW57Uq6?WMLMf&aBm;(j#({ViGZ1i!4BiSH;pEYb0Et!vi3l4s7Ct2ixgp%U86Ay- zMC0%;%3vuQ>|dO94(SqL^We1fQ2&_+(dHQ_{4g;*kf2h8!xrm#ju73TfX12QAxf~25M9V57DW&t zgdzwZnMBA9;hrh6Sp(u(WbYGT6KgQc7_bEp4w#9aP9RcnlG1_!Tk#1uoKVq(CC#F` z8N{PX0f2B4ghgzqf*e3n#vs;Uurok`54Hi!BNSR-MIbF;2AlvJfEmjWVJL|d&=3*0 zZd3+dNmQi}9!cJXb<-ePiLr|ozC{WP(wW3K8sa^iB-jX?vY7Kr_|iZ2u~F=zU@IvN zP_Q8+H&!6Jh$2T@-Z+aJaKgdN=(vM*G;`t(8-&bY2_=M!aI(ND0H-XX=s;Bh)-}x2 z4Z_DG!MCK>TSz)c4j^#Y4whU3aXpko_zLXC(Rv9I3>YM(wLV3Sd<@r+98KWZh8P28 z5-7`P;afyXBZEbZqaofyCP#RVLyu%)!X0No$; zP?X^7)x#nRqz-3q5T*#3Cbr6;nZaPkfa+hc4Pf5rs1_s`kVsGfA#ou!1-5()4n-J& zl+qwVC`KU3lFr0p9*W`EIS^f75?^Y@<}q+r6}zbfM3FXrA^Zt84(t#t1VR}REQo$A zZono15d)LMtzrVT_%T9(i0}jH!x`b&B`~Z(761n>3V|$(!bYe<5+-CNSQCDrNQ#nEriV|WRjZ+Q8Oce7-<$#UAqX{gJL<~soKm-70 zc13j?LM0&!ixltFG_aVLk z`vKZd9}Q(#XrR*YqzDy;g&;l}HFTjGK@?62kR(nF(S=MBp%haIk^w|E-|+bnT@GS8 z%vKPODk%zPAdDuqcmg?qCR0ion88CD+0;t$V^BT4oKo^7f9Ze&Mi&Fw5 zi4#L~A(N;|(XGeLhnRt?8_dHd0TBa}LpKJAEy}>50E%vqe{lyO78#`G2}m!IaKC7p@IJeU`WrNKtvQ4E$xBJfp3NP&c8B6d+E1vr>U)3#vyaVWznOje?R zm}Bh%RDCbmxODB|zZxH%Xu_^%vA3W27c=!?% zEM`FJ@HXio3Xp7s2w;#f12JfXN+c;lEn~1IB=uBf!UG9vDw5}LGQkc)*9g*&QvxK3 z6GL<%lSC-RRDxu{@Xt69&te7wPLaV|fg_wenh_w;iXahTgT}(A1R*zsdpDrr88koR zERT>RAbv;F2;$G+5M9V5s!}AI2{0jMz;&Z>Vd4;BD2ZeuL@SQS1BW~EAZY>;14NO7x9WmqJS-l9 zNRUZlF^`CF8Z4nigj2yD#G?)@kCA{NqIk5SOAVF~fjAsWBBcq42)e7Ge7urSH7L{& z^es|Ykj^AIfWVt5s2Erti5N|nLvW2UIR22*7nqNl*pXyOXJRoA zntX_+z((Lv4VFhDM$_RCOox#8C98Q(dZNUs1Coj$Zh=%E_!1Mcy3u(foPi3LKn-13 z=z&Ih!7@k!L@~jdPzapb5UMbhAmoPda5=FV2jW@GK)@+Ntidn?z!pF_U?yr+0+B!> zY0+*%7*C7~5UP`sNbY%qEr6H+W)6UF;kk<(1~HC?cn_Hz31sEG{3qe`WU5}M@ZT~ho(KrP5)h*kiIM-^{^3>w{MhkNJ_5WgtO> aA`lN1BjiA=N)3&qqzX-qBux+yU;qH}K$ngH literal 0 HcmV?d00001 From cfcc0059bfb8be7d1da80cb6a75d9ba71f4662f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 11 Apr 2019 14:02:59 +0200 Subject: [PATCH 2/3] sd-bus: add define for the maximum signature length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Less magic numbers in the code… --- src/libsystemd/sd-bus/bus-message.c | 2 +- src/libsystemd/sd-bus/bus-signature.c | 2 +- src/systemd/sd-bus.h | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index a2464e1a46..427d42f296 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -284,7 +284,7 @@ static int message_append_field_signature( /* dbus1 doesn't allow signatures over 8bit, let's enforce * this globally, to not risk convertability */ l = strlen(s); - if (l > 255) + if (l > SD_BUS_MAXIMUM_SIGNATURE_LENGTH) return -EINVAL; /* Signature "(yv)" where the variant contains "g" */ diff --git a/src/libsystemd/sd-bus/bus-signature.c b/src/libsystemd/sd-bus/bus-signature.c index 1ecd6e8b7e..b420ba3688 100644 --- a/src/libsystemd/sd-bus/bus-signature.c +++ b/src/libsystemd/sd-bus/bus-signature.c @@ -144,5 +144,5 @@ bool signature_is_valid(const char *s, bool allow_dict_entry) { p += t; } - return p - s <= 255; + return p - s <= SD_BUS_MAXIMUM_SIGNATURE_LENGTH; } diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 129cc93328..311602d048 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -33,6 +33,9 @@ _SD_BEGIN_DECLARATIONS; #define SD_BUS_DEFAULT_USER ((sd_bus *) 2) #define SD_BUS_DEFAULT_SYSTEM ((sd_bus *) 3) +/* https://dbus.freedesktop.org/doc/dbus-specification.html#message-protocol-marshaling-signature */ +#define SD_BUS_MAXIMUM_SIGNATURE_LENGTH 255 + /* Types */ typedef struct sd_bus sd_bus; From fb270a26b20e5e3997fb88ffa5e257af194a6cb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 11 Apr 2019 14:07:22 +0200 Subject: [PATCH 3/3] sd-bus: add define for the maximum name length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Less magic numbers in the code… --- src/libsystemd/sd-bus/bus-internal.c | 6 +++--- src/systemd/sd-bus.h | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-internal.c b/src/libsystemd/sd-bus/bus-internal.c index 598b7f110c..dff39cb13f 100644 --- a/src/libsystemd/sd-bus/bus-internal.c +++ b/src/libsystemd/sd-bus/bus-internal.c @@ -97,7 +97,7 @@ bool interface_name_is_valid(const char *p) { dot = false; } - if (q - p > 255) + if (q - p > SD_BUS_MAXIMUM_NAME_LENGTH) return false; if (dot) @@ -139,7 +139,7 @@ bool service_name_is_valid(const char *p) { dot = false; } - if (q - p > 255) + if (q - p > SD_BUS_MAXIMUM_NAME_LENGTH) return false; if (dot) @@ -170,7 +170,7 @@ bool member_name_is_valid(const char *p) { return false; } - if (q - p > 255) + if (q - p > SD_BUS_MAXIMUM_NAME_LENGTH) return false; return true; diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 311602d048..84ceb62dc7 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -36,6 +36,9 @@ _SD_BEGIN_DECLARATIONS; /* https://dbus.freedesktop.org/doc/dbus-specification.html#message-protocol-marshaling-signature */ #define SD_BUS_MAXIMUM_SIGNATURE_LENGTH 255 +/* https://dbus.freedesktop.org/doc/dbus-specification.html#message-protocol-names */ +#define SD_BUS_MAXIMUM_NAME_LENGTH 255 + /* Types */ typedef struct sd_bus sd_bus;