Hi Raphael,
On Thu, Apr 06, 2017 at 06:02:41PM +0200, Raphael Hertzog wrote:
> Hi,
>
> On Mon, 03 Apr 2017, Salvatore Bonaccorso wrote:
> > CVE-2016-10209[0]:
> > | The archive_wstring_append_from_mbs function in archive_string.c in
> > | libarchive 3.2.2 allows remote attackers to cause a denial of service
> > | (NULL pointer dereference and application crash) via a crafted archive
> > | file.
> >
> > It was reported upstream at [1] and if I'm correct the fix should be
> > [2]. Can you confirm that?
> >
> > [1] https://github.com/libarchive/libarchive/issues/842
> > [2]
> > https://github.com/libarchive/libarchive/commit/42a3408ac7df1e69bea9ea12b72e14f59f7400c0
>
> I tried to reproduce the issue on all Debian versions and using "bsdtar"
> from "libarchive-tools" (in stretch 3.2.1-6 and sid 3.2.2-2) or from
> "bsdtar" (in jessie 3.1.2-11+deb8u3 / wheezy 3.0.4-3+wheezy5+deb7u1), I always
> get the same error:
>
> $ wget
> https://frankowicz.me/storage/crashes/la_segv_archive_wstring_append_from_mbs
> -O CVE-2016-10209-la_segv_archive_wstring_append_from_mbs
> $ bsdtar -t -f CVE-2016-10209-la_segv_archive_wstring_append_from_mbs
> bsdtar: Archive entry has empty or unreadable filename ... skipping.
> bsdtar: (null)
> bsdtar: Error exit delayed from previous errors.
>
> Thus I don't have any segfault similar to what is reported in [1].
> I'm not sure what conclusion I should draw from this...
>
> My tests have been made on amd64.
what follows is not yet a full analysis. But the problem seems hided.
In the upstream report the issue is raised at
577 archive_wstring_append_from_mbs(struct archive_wstring *dest,
578 const char *p, size_t len)
[...]
588 const char *mbs = p;
[...]
603 while (*mbs && mbs_length > 0) {
load in debugger, and setting a break to the repsecitve function
archive_wstring_append_from_mbs, if it get's p=0x0, there the null
pointer will be dereferenced.
Now the upper function is archive_mstring_get_wcs, and if aes->aes_set is
AES_SET_MBS and aes->aes_mbs.s is NULL, then the above situation arise.
----cut---------cut---------cut---------cut---------cut---------cut-----
archive_mstring_get_wcs(struct archive *a, struct archive_mstring *aes,
const wchar_t **wp)
{
int r, ret = 0;
(void)a;/* UNUSED */
/* Return WCS form if we already have it. */
if (aes->aes_set & AES_SET_WCS) {
*wp = aes->aes_wcs.s;
return (ret);
}
*wp = NULL;
/* Try converting MBS to WCS using native locale. */
if (aes->aes_set & AES_SET_MBS) {
archive_wstring_empty(&(aes->aes_wcs));
r = archive_wstring_append_from_mbs(&(aes->aes_wcs),
aes->aes_mbs.s, aes->aes_mbs.length);
if (r == 0) {
aes->aes_set |= AES_SET_WCS;
*wp = aes->aes_wcs.s;
} else
ret = -1;/* failure. */
}
return (ret);
}
----cut---------cut---------cut---------cut---------cut---------cut-----
The upstream commit
https://github.com/libarchive/libarchive/commit/42a3408ac7df1e69bea9ea12b72e14f59f7400c0
now enforces that, "This ensures e.g. that
archive_mstring_copy_mbs_len_l() does not set aes_set = AES_SET_MBS
with aes_mbs.s == NULL."
If it can be proven that the situation for Debian binary packages this
is never ever possible (why?), then I'm fine to mark the issue in the
security-tracker to unimportant, since the vulnerable source is still
there.
Regards,
Salvatore
