<html>
<head>
<base href="https://bugs.freedesktop.org/">
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Buffer overflow in gallium/auxiliary/hud/hud_cpufreq.c"
href="https://bugs.freedesktop.org/show_bug.cgi?id=105274#c3">Comment # 3</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Buffer overflow in gallium/auxiliary/hud/hud_cpufreq.c"
href="https://bugs.freedesktop.org/show_bug.cgi?id=105274">bug 105274</a>
from <span class="vcard"><a class="email" href="mailto:gw.fossdev@gmail.com" title="Gert Wollny <gw.fossdev@gmail.com>"> <span class="fn">Gert Wollny</span></a>
</span></b>
<pre>The sscanf will accept the number and not check the rest of the string, yes,
but later in the code you have
char basename[256];
snprintf(basename, sizeof(basename),
"/sys/devices/system/cpu/%s", dp->d_name);
so basename is limited to 256 chars including the terminator 0, and then
snprintf(fn, sizeof(fn), "%s/cpufreq/scaling_cur_freq", basename);
with sizeof(fn)== 128, which means fn is not longer then 128 byte, just like it
is defined in cpufreq_info::sysfs_filename, hence no buffer overflow there.
Then
if (stat(fn, &stat_buf) < 0)
continue;
must pass, which means whatever string was created before, it must still point
to a valid file name that at this point is
/sys/devices/system/cpu/??/cpufreq/scaling_cur_freq
and this limits the size of ?? == dp->d_name to less than 80 characters (This,
by the way, also means one has to create this special file, it is not
sufficient to add an arbitrary string to /sys/devices/system/cpu/cpuN). So
yes, one can get a buffer overflow in cpufreq_info::name but it can not
overwrite anything important, because given the memory layout of cpufreq_info
it may temporarily overwrite cpufreq_info::cpu_index and a part of
cpufreq_info::sysfs_filename, but these are set later in add_object anyway, and
because of cpufreq_info::sysfs_filename being a null terminated string,
cpufreq_info::name would also always be null-termianted, limiting the impact of
its later use.
IMHO the better fix is snprintf, because it creates a null terminated string
and doesn't do any post-hoc error handling. Unless someone beats me to it I'll
send a patch tomorrow.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
<li>You are the QA Contact for the bug.</li>
</ul>
</body>
</html>