| Message ID | d3e5398c0adb37f80ace13fbb5cd6518dc1fba6b.1535708312.git.sbrivio@redhat.com |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [ipset,v3] Validate string type attributes in attr2data() | expand |
On Fri, 31 Aug 2018, Stefano Brivio wrote: > Otherwise, we are missing checks in some paths, e.g. we might > overrun the buffer used to save the set name in callback_list() > when we strcpy() to it. > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > --- > v3: Also as pointed out by Jozsef, there's no need to validate > the set name in ipset_cmd(), this is done already while > parsing the command line, so drop that part and change the > patch title accordingly. > > v2: As requested by Jozsef, move validation of setname length to > attr2data() for data received via netlink, instead of doing > it in callback_list(). > > lib/session.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/lib/session.c b/lib/session.c > index ca96aaa57ea6..16b5549e73db 100644 > --- a/lib/session.c > +++ b/lib/session.c > @@ -678,6 +678,10 @@ attr2data(struct ipset_session *session, struct nlattr *nla[], > default: > break; > } > + } else if (attr->type == MNL_TYPE_NUL_STRING) { > + if (!d || strlen(d) >= attr->len) > + FAILURE("Broken kernel message: " > + "string type attribute missing or too long!"); > } > #ifdef IPSET_DEBUG > else > -- Patch is applied, thanks. Best regards, Jozsef - E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences H-1525 Budapest 114, POB. 49, Hungary
diff --git a/lib/session.c b/lib/session.c index ca96aaa57ea6..16b5549e73db 100644 --- a/lib/session.c +++ b/lib/session.c @@ -678,6 +678,10 @@ attr2data(struct ipset_session *session, struct nlattr *nla[], default: break; } + } else if (attr->type == MNL_TYPE_NUL_STRING) { + if (!d || strlen(d) >= attr->len) + FAILURE("Broken kernel message: " + "string type attribute missing or too long!"); } #ifdef IPSET_DEBUG else
Otherwise, we are missing checks in some paths, e.g. we might overrun the buffer used to save the set name in callback_list() when we strcpy() to it. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> --- v3: Also as pointed out by Jozsef, there's no need to validate the set name in ipset_cmd(), this is done already while parsing the command line, so drop that part and change the patch title accordingly. v2: As requested by Jozsef, move validation of setname length to attr2data() for data received via netlink, instead of doing it in callback_list(). lib/session.c | 4 ++++ 1 file changed, 4 insertions(+)