| Message ID | 1365504751-10093-1-git-send-email-luis.henriques@canonical.com |
|---|---|
| State | New |
| Headers | show |
Hit 'send' too soon... Forgot to specify this is for the Lucid kernel, to fix CVE-2013-0349. On Tue, Apr 09, 2013 at 11:52:31AM +0100, Luis Henriques wrote: > From: Anderson Lizardo <anderson.lizardo@openbossa.org> > > CVE-2013-0349 > > BugLink: http://bugs.launchpad.net/bugs/1134503 > > The length parameter should be sizeof(req->name) - 1 because there is no > guarantee that string provided by userspace will contain the trailing > '\0'. > > Can be easily reproduced by manually setting req->name to 128 non-zero > bytes prior to ioctl(HIDPCONNADD) and checking the device name setup on > input subsystem: > > $ cat /sys/devices/pnp0/00\:04/tty/ttyS0/hci0/hci0\:1/input8/name > AAAAAA[...]AAAAAAAAf0:af:f0:af:f0:af > > ("f0:af:f0:af:f0:af" is the device bluetooth address, taken from "phys" > field in struct hid_device due to overflow.) > > Cc: stable@vger.kernel.org > Signed-off-by: Anderson Lizardo <anderson.lizardo@openbossa.org> > Acked-by: Marcel Holtmann <marcel@holtmann.org> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > (back ported from commit 0a9ab9bdb3e891762553f667066190c1d22ad62b) > > Signed-off-by: Luis Henriques <luis.henriques@canonical.com> > --- > net/bluetooth/hidp/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c > index 49d8495..0c2c59d 100644 > --- a/net/bluetooth/hidp/core.c > +++ b/net/bluetooth/hidp/core.c > @@ -778,7 +778,7 @@ static int hidp_setup_hid(struct hidp_session *session, > hid->version = req->version; > hid->country = req->country; > > - strncpy(hid->name, req->name, 128); > + strncpy(hid->name, req->name, sizeof(req->name) - 1); > strncpy(hid->phys, batostr(&src), 64); > strncpy(hid->uniq, batostr(&dst), 64); > > -- > 1.8.1.2 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team Cheers, -- Luis
On 09/04/13 11:54, Luis Henriques wrote: > Hit 'send' too soon... Forgot to specify this is for the Lucid kernel, > to fix CVE-2013-0349. > > On Tue, Apr 09, 2013 at 11:52:31AM +0100, Luis Henriques wrote: >> From: Anderson Lizardo <anderson.lizardo@openbossa.org> >> >> CVE-2013-0349 >> >> BugLink: http://bugs.launchpad.net/bugs/1134503 >> >> The length parameter should be sizeof(req->name) - 1 because there is no >> guarantee that string provided by userspace will contain the trailing >> '\0'. >> >> Can be easily reproduced by manually setting req->name to 128 non-zero >> bytes prior to ioctl(HIDPCONNADD) and checking the device name setup on >> input subsystem: >> >> $ cat /sys/devices/pnp0/00\:04/tty/ttyS0/hci0/hci0\:1/input8/name >> AAAAAA[...]AAAAAAAAf0:af:f0:af:f0:af >> >> ("f0:af:f0:af:f0:af" is the device bluetooth address, taken from "phys" >> field in struct hid_device due to overflow.) >> >> Cc: stable@vger.kernel.org >> Signed-off-by: Anderson Lizardo <anderson.lizardo@openbossa.org> >> Acked-by: Marcel Holtmann <marcel@holtmann.org> >> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> >> (back ported from commit 0a9ab9bdb3e891762553f667066190c1d22ad62b) >> >> Signed-off-by: Luis Henriques <luis.henriques@canonical.com> >> --- >> net/bluetooth/hidp/core.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c >> index 49d8495..0c2c59d 100644 >> --- a/net/bluetooth/hidp/core.c >> +++ b/net/bluetooth/hidp/core.c >> @@ -778,7 +778,7 @@ static int hidp_setup_hid(struct hidp_session *session, >> hid->version = req->version; >> hid->country = req->country; >> >> - strncpy(hid->name, req->name, 128); >> + strncpy(hid->name, req->name, sizeof(req->name) - 1); >> strncpy(hid->phys, batostr(&src), 64); >> strncpy(hid->uniq, batostr(&dst), 64); >> >> -- >> 1.8.1.2 >> >> >> -- >> kernel-team mailing list >> kernel-team@lists.ubuntu.com >> https://lists.ubuntu.com/mailman/listinfo/kernel-team > > Cheers, > -- > Luis > Simple one liner from upstream. looks fine to me. Acked-by Colin Ian King <colin.king@canonical.com>
On Tue, Apr 09, 2013 at 11:54:37AM +0100, Luis Henriques wrote: > Hit 'send' too soon... Forgot to specify this is for the Lucid kernel, > to fix CVE-2013-0349. > > On Tue, Apr 09, 2013 at 11:52:31AM +0100, Luis Henriques wrote: > > From: Anderson Lizardo <anderson.lizardo@openbossa.org> > > > > CVE-2013-0349 > > > > BugLink: http://bugs.launchpad.net/bugs/1134503 > > > > The length parameter should be sizeof(req->name) - 1 because there is no > > guarantee that string provided by userspace will contain the trailing > > '\0'. > > > > Can be easily reproduced by manually setting req->name to 128 non-zero > > bytes prior to ioctl(HIDPCONNADD) and checking the device name setup on > > input subsystem: > > > > $ cat /sys/devices/pnp0/00\:04/tty/ttyS0/hci0/hci0\:1/input8/name > > AAAAAA[...]AAAAAAAAf0:af:f0:af:f0:af > > > > ("f0:af:f0:af:f0:af" is the device bluetooth address, taken from "phys" > > field in struct hid_device due to overflow.) > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Anderson Lizardo <anderson.lizardo@openbossa.org> > > Acked-by: Marcel Holtmann <marcel@holtmann.org> > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > (back ported from commit 0a9ab9bdb3e891762553f667066190c1d22ad62b) > > > > Signed-off-by: Luis Henriques <luis.henriques@canonical.com> > > --- > > net/bluetooth/hidp/core.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c > > index 49d8495..0c2c59d 100644 > > --- a/net/bluetooth/hidp/core.c > > +++ b/net/bluetooth/hidp/core.c > > @@ -778,7 +778,7 @@ static int hidp_setup_hid(struct hidp_session *session, > > hid->version = req->version; > > hid->country = req->country; > > > > - strncpy(hid->name, req->name, 128); > > + strncpy(hid->name, req->name, sizeof(req->name) - 1); > > strncpy(hid->phys, batostr(&src), 64); > > strncpy(hid->uniq, batostr(&dst), 64); > > > > -- > > 1.8.1.2 > > > > > > -- > > kernel-team mailing list > > kernel-team@lists.ubuntu.com > > https://lists.ubuntu.com/mailman/listinfo/kernel-team Looks simple. 'name' is a char name[128] jobbie so this looks safe to me. Acked-by: Andy Whitcroft <apw@canonical.com> -apw
Applied to Lucid. -apw
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index 49d8495..0c2c59d 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -778,7 +778,7 @@ static int hidp_setup_hid(struct hidp_session *session, hid->version = req->version; hid->country = req->country; - strncpy(hid->name, req->name, 128); + strncpy(hid->name, req->name, sizeof(req->name) - 1); strncpy(hid->phys, batostr(&src), 64); strncpy(hid->uniq, batostr(&dst), 64);