In R 2.6.1, a couple of places (discovered using valgrind) where the requested size of string buffers fails to account correctly for the trailing null byte: 1. In src/appl/strsignif.c, 'f0' and 'form' at l. 108-9 each need at least 1 extra byte. 2. In src/main/util.c, 'out' at l. 1081 needs at least one extra byte. (Remember that the return value of strlen does not include the null byte.) Andrew Runnalls
A.R.Runnalls at kent.ac.uk wrote:> In R 2.6.1, a couple of places (discovered using valgrind) where the > requested size of string buffers fails to account correctly for the > trailing null byte: > > 1. In src/appl/strsignif.c, 'f0' and 'form' at l. 108-9 each need at > least 1 extra byte. >At least this one is still there; I'll fix it (and take a look at the other, too). Thanks! Duncan Murdoch> 2. In src/main/util.c, 'out' at l. 1081 needs at least one extra byte. > > (Remember that the return value of strlen does not include the null byte.) > > Andrew Runnalls > > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel >
On Fri, 25 Jan 2008, A.R.Runnalls at kent.ac.uk wrote:> In R 2.6.1, a couple of places (discovered using valgrind) where the > requested size of string buffers fails to account correctly for the > trailing null byte: > > 1. In src/appl/strsignif.c, 'f0' and 'form' at l. 108-9 each need at > least 1 extra byte. > > 2. In src/main/util.c, 'out' at l. 1081 needs at least one extra byte. > > (Remember that the return value of strlen does not include the null byte.)But it is subtler than that. R_alloc contains the statement s = allocVector(RAWSXP, size + 1); and so does over-allocate by at least one (there is a rounding up to a multiple of 8). This is a historical anomaly (it used to allocate a CHARSXP that allowed for the null byte), but one which trying to eliminate caused too many crashes in package code. I'd like to see the empirical evidence you have, as I have been unable to trigger an overrun here. -- Brian D. Ripley, ripley at stats.ox.ac.uk Professor of Applied Statistics, http://www.stats.ox.ac.uk/~ripley/ University of Oxford, Tel: +44 1865 272861 (self) 1 South Parks Road, +44 1865 272866 (PA) Oxford OX1 3TG, UK Fax: +44 1865 272595
Prof Brian Ripley wrote:> On Fri, 25 Jan 2008, A.R.Runnalls at kent.ac.uk wrote: > >> In R 2.6.1, a couple of places (discovered using valgrind) where the >> requested size of string buffers fails to account correctly for the >> trailing null byte: >> >> 1. In src/appl/strsignif.c, 'f0' and 'form' at l. 108-9 each need at >> least 1 extra byte. >> >> 2. In src/main/util.c, 'out' at l. 1081 needs at least one extra byte. >> >> (Remember that the return value of strlen does not include the null >> byte.) > > But it is subtler than that. R_alloc contains the statement > > s = allocVector(RAWSXP, size + 1); > > and so does over-allocate by at least one (there is a rounding up to a > multiple of 8). This is a historical anomaly (it used to allocate a > CHARSXP that allowed for the null byte), but one which trying to > eliminate caused too many crashes in package code. > > I'd like to see the empirical evidence you have, as I have been unable > to trigger an overrun here.I should perhaps have explained that I discovered these faults working on a modified code base, in which in particular the valgrind instrumentation will pick up an overrun of even a single byte over the requested block size. I have no reason to suppose that the faults will lead to a failure of R as currently engineered; indeed for the reasons you cite I guess they won't. However, buffer overruns - when they do manifest themselves - can give rise to such baffling and hard-to-reproduce failures that they are like submerged rocks: well worth marking on the chart even if they're not close to current shipping lanes. Andrew Runnalls -- Dr Andrew Runnalls, Computing Laboratory, University of Kent, CANTERBURY CT2 7NF, UK Tel: (0)1227 823821