On 01/06/2015 09:58, Markus Armbruster wrote:
>> > - snprintf(s->smb_dir, sizeof(s->smb_dir), "/tmp/qemu-smb.%ld-%d",
>> > - (long)getpid(), instance++);
>> > - if (mkdir(s->smb_dir, 0700) < 0) {
>> > - error_report("could not create samba server dir '%s'",
>> > s->smb_dir);
>> > + if (!(tmpdir = mkdtemp(smb_dir))) {
mkdtemp is unfortunately missing on Windows, and g_mkdtemp requires glib
2.30.
Paolo
>> > + error_report("could not create samba server dir '%s'", smb_dir);
>> > return -1;
>> > }
>> > + strncpy(s->smb_dir, tmpdir, sizeof(s->smb_dir));
> We tend to eschew strncpy() and use our (slightly less bad) pstrcpy().
> Recommend to use it here, too.
>
>> > snprintf(smb_conf, sizeof(smb_conf), "%s/%s", s->smb_dir,
>> > "smb.conf");
>> >
>> > f = fopen(smb_conf, "w");
> Michael (cc'ed) already posted "[PATCH] slirp: use less predictable
> directory name in /tmp for smb config (CVE-2015-4037)"[*]. His patch
> clobbers s->smb_dir[] when mkdtemp() fails (missed that in my review),
> yours doesn't.
>
> Suggest you guys figure out together which solution you want.
>
> Preferably with strncpy() replaced by pstrcpy():
> Reviewed-by: Markus Armbruster <arm...@redhat.com>
