On Wed, Jul 29, 2009 at 03:42:05PM -0400, Jason A. Smith wrote:
>
> In gmond, the monitor-core/libmetrics/linux/metrics.c:find_disk_space()
> function, was not only using small character arrays, but the arrays for
> the sscanf after the fgets were smaller than the array for the line it
> just read in, which can lead to buffer overflows and the "stack
> smashing" problem that we were having.
using fixed size arrays in the stack is never a good idea.
in this case it could be theoretically possible to exploit this overflow
with the help of a malicious NFS server (very unlikely though).
> To fix out problem and prevent the overflows, I made a patch to increase
> the size of the arrays and also make each of the arrays used in the
> sscanf the same size as the line buffer used in fgets, so there is no
> chance of another overflow.
committed for trunk in r2007, but the new implementation might also
generate segfaults on its own due to stack overflows when running with
very small stacks as it requires a bigger stack.
IMHO it would be better to migrate this function to use getmntent and
friends as it was done already for Cygwin, Solaris and the BSD and that
way avoid the use of local buffers and parsing of the /proc/mounts file
directly.
Carlo
>
> ~Jason
>
>
> --
> /------------------------------------------------------------------\
> | Jason A. Smith Email: smit...@bnl.gov |
> | Atlas Computing Facility, Bldg. 510M Phone: +1-631-344-4226 |
> | Brookhaven National Lab, P.O. Box 5000 Fax: +1-631-344-7616 |
> | Upton, NY 11973-5000, U.S.A. |
> \------------------------------------------------------------------/
>
> Index: monitor-core/libmetrics/linux/metrics.c
> ===================================================================
> --- monitor-core/libmetrics/linux/metrics.c (revision 2006)
> +++ monitor-core/libmetrics/linux/metrics.c (working copy)
> @@ -1202,8 +1202,8 @@
> float find_disk_space(double *total_size, double *total_free)
> {
> FILE *mounts;
> - char procline[256];
> - char mount[128], device[128], type[32], mode[128];
> + char procline[1024];
> + char device[1024], mount[1024], type[1024], mode[1024];
> /* We report in GB = 1e9 bytes. */
> double reported_units = 1e9;
> /* Track the most full disk partition, report with a percentage. */
> ------------------------------------------------------------------------------
> Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
> trial. Simplify your report design, integration and deployment - and focus on
> what you do best, core application coding. Discover what's new with
> Crystal Reports now. http://p.sf.net/sfu/bobj-july
> _______________________________________________
> Ganglia-developers mailing list
> Ganglia-developers@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ganglia-developers
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Ganglia-developers mailing list
Ganglia-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ganglia-developers
