search

hughbarney / atto

Atto Emacs is a minimal functional Emacs in less than 2000 lines of C code. It is derived from Anthony's Editor and uses a buffer-gap to represent the file in memory.
119 stars 25 forks source link

Filename length is constrained. #2

Closed skx closed 8 years ago

skx commented 8 years ago

The maximum length of a filename is limited to 255, and exceeding that will cause a buffer-overflow:

   helsinki ~/atto $ ./atto $(perl -e 'print "X" x3333')
   Segmentation fault

Suggest we change b_fname in header.h to a char * instead of a fixed buffer. Then rather than testing if strlen(b_fname)>0, in get_buffer_name() we can test for != NULL, etc.

I'm happy to contribute a pull-request, if you wish. It should be easy to use malloc/free and similar. Hardest part will be avoiding the whitespace changes ;)

hughbarney commented 8 years ago

Yes. Definite overflow bug. Not sure I want to fix it using malloc() / free() as that would imply no limit. I tested on a few other small emacs editors, (Mg, Zile, MicroEmacs, ErsatzEmacs) and each of these behaves slightly differently. Only GNU Emacs will fully handle your test case. The standard approach seems to be to limit the filelength to around 256 chars and truncate. My gut feeling is to fix this by using strncpy instead of strcpy; and check all other instances of code for similar overflows. This will mean no change to the line count.

Watch this space as I will be pushing another bug fix in the next day or so and will fix this issue then.

I'm interested how you came to use that particular test case ? Also what OS are you running / building on ?

Thanks Hugh

skx commented 8 years ago

I guess there's a difference between "unlimited" and "long enough to contain all sane filenames". If you were being portable you'd include the appropriate header and use PATH_MAX which on my system gives me 4096 bytes for holding the longest-possible filename.

I'm interested how you came to use that particular test case ?

I was looking over the code to see how hard it would be to add TAB-completion, after taking it for a spin and finding it pretty functional. Once I saw the static buffer I had to test the behaviour - I'd hoped for a clean error of the form "path too long", but a segfault happened instead.

Also what OS are you running / building on ?

Linux. Specifically the current stable release of Debian (jessie), on an amd64 system.

hughbarney commented 8 years ago

Yes PATH_MAX would be the sensible approach.

Adding TAB completion would be fairly easy to do in about 100-150 lines of code. I have an implementation that I borrowed from MicroEMACS 3.11 in my other github project called Perfect Emacs (pemacs). That implementation uses Termcap but it would simple to change.

I considered doing this but it would definitely blow over the 2000 line limit I have set on this project and I am very keen to stick to that limit otherwise the temptation is to keep adding features. I wanted Atto to be functional, but small and lean so that it was easy to understand. I would definitely add it to "Femto" Emacs (if I ever get round to it) Have a look at Perfect Emacs if you fancy doing a fork.

hughbarney commented 8 years ago

Fixed in Atto 1.6. Filename is limited to NAME_MAX (255 chars) from