RISC OS Open
A fast and easily customised operating system for ARM devices
ROOL
Home | News | Downloads | Bugs | Bounties | Forum | Documents | Photos | Contact us
Account

Previous|Next

  • Tickets
  • » Ticket #345

Ticket #345 (Invalid)Sat May 25 18:13:05 UTC 2013

Potential DOSFS buffer overrun when handling long filenames?

Reported by: Rick Murray (539) Severity: Normal
Part: RISC OS: Module Release:
Milestone: Status Invalid

Details by Rick Murray (539):

Referring to the source file <a href=“https://www.riscosopen.org/viewer/view/castle/RiscOS/Sources/FileSys/ImageFS/DOSFS/c/DOSnaming?rev=4.7;content-type=text%2Fplain”>DOSnaming</a>.

There are two functions of note. Here is the first:

<pre><code>/-————————————————————————————————————/
/* Check that the given DOS names are identical */

int namematch(char *wcname,char *fname)
{
char string1257;
char string2257 ;

/* Code assumes characters upto (and including) “file_sep” will always fit
  • in RISC OS names.
    /
    before(string1,fname,file_sep,0) ;
    before(string2,wcname,file_sep,0) ;
    if (wild_card_compare(string1,string2,DOSwcmult,DOSwcsing) == TRUE)
    {
    /
    “string1” is the full (non-wildcarded) filename we have matched with /
    /
    “string2” is the original (wildcarded) filename we were given on entry */
after(string1,fname,file_sep,0) ; after(string2,wcname,file_sep,0) ; if (wild_card_compare(string1,string2,DOSwcmult,DOSwcsing) == TRUE) return(TRUE) ; } return(FALSE) ;

}</code></pre>

and here is the second:

<pre><code>/-———————————————————————————————————/
/* return a string containing the text before the given character */

char *before(char *newptr,char *text,char marker,int npad)
{
int cpos = chr_pos(text,marker) ;
if (cpos == 0)
cpos = strlen(text) ;

strncpy(newptr,text,cpos) ; if (npad == 0) newptr[cpos] = NULL ; return(newptr) ;

}</code></pre>

What comes to mind is what happens if the filename is <i>longer</i> than 256 characters before the separator? The string1 and string2 chars are only large enough for 256, and the before() function will, if there isn’t a separator, default to the length of the text string (or if there is, to whatever size that is including >256).
In short, passing a stupid-long filename to DOSFS will splatter gooey bits of gunk all over places it shouldn’t.

To fix, not difficult, something like:

Make the filename buffers appropriately long (might be a FAT issue?) and include a check in before() to ensure that this buffer is not overshot.

There is a routine called after() that may need to be checked as well.

Changelog:

Modified by Rick Murray (539) Sat, May 25 2013 - 18:14:20 GMT

I hate Textile. :-(

Modified by Sprow (202) Mon, May 27 2013 - 08:56:53 GMT

  • Status changed from Open to Invalid

The namematch() function is only ever called with the leafname of the directory entry to be compared. The maximum long filename length looks to be 255, so there’s no immediate overflow problem.

When I say immediate, they’re actually 255 x UTF16 characters, but DOSFS brutally mangles the names into ASCII at present.

  • Comment on, or change status of, this ticket

Previous|Next

Search tickets

Social

Follow us on and

ROOL Store

Buy RISC OS Open merchandise here, including SD cards for Raspberry Pi and more.

Donate! Why?

Help ROOL make things happen – please consider donating!

RISC OS IPR

RISC OS is an Open Source operating system owned by RISC OS Developments Ltd and licensed primarily under the Apache 2.0 license.

Options

  • Tickets
  • New ticket
  • Milestones
  • Subversion: Changesets
  • Subversion: Browse
  • CVS: Revisions
  • CVS: Browse
  • Search

RSS feeds Rss

  • Tickets
  • Everything!
  • More feeds...
Site design © RISC OS Open Limited 2018 except where indicated
The RISC OS Open Collaboa theme is distantly based on the Collaboa default layout

Valid XHTML 1.0  |  Valid CSS

Powered by Collaboa
This site runs on Rails

Hosted by Arachsys