headers
John Beckett | 1 May 2007 11:44
Picon
Favicon

Re: feedkeys() allowed in sandbox

Bram Moolenaar wrote:
> N times as safe still isn't 100% safe.

I am not claiming that sanity-checking a modeline before
execution would make it 100% safe. But there have been many
examples in other software where minor bugs have turned into
security disasters because some simple point that could have
been checked, wasn't.

While code is working correctly, a simple check is redundant,
and indeed is offensive because it lengthens and obscures the
code. But a few simple checks may prevent disaster at some
future time, when Vim is further developed.

The Google test (searching for past instances of trouble with
Vim's modeline) proves the case that future problems are likely.

> Modelines are default off when you are root.
> The mail filetype plugin also switches it off.

Good grief - I didn't know that. So you *have* got sanity checks
built in! I'll go and sit in the corner now, but thanks for
confirming that multiple layers of defence are desirable.

John
Permalink | Reply | 0 comments |
headers
John Beckett | 1 May 2007 11:42
Picon
Favicon

Re: feedkeys() allowed in sandbox

Matthew Winn wrote:
> If there was a security problem in Vim do you really think it
> couldn't be exploited in 100 characters? That's a pretty shaky
> foundation on which to build your security.

I am quite surprised at the lack of appreciation for the merits
of "defense in depth" here. I am not claiming that a length
limit would preclude damage, just that a modeline should be
sanity checked before execution, and a reasonable length would
be the first check.

It's certainly true that a modeline of 100 bytes could wreak
havoc on an unpatched Vim. But it is quite possible that a
future vulnerability might allow code to be injected from a
modeline, and limiting its length *might* make the attacker's
job harder.

It's sensational that Vim can process files with very long
lines, for the occasions we need that. But it would be absurd
for Vim to process a multi-megabyte modeline.

By all means abuse me for my cheeky suggestion to limit
modelines to 100 bytes, but while doing that you might agree
that some limit under 1MB should be enforced.

> A web browser should be able to handle anything thrown at it
> in a way that doesn't compromise security. _Every_ application
> should be able to handle anything thrown at it in a way that
> doesn't compromise security.
(Continue reading)

Permalink | Reply | 4 comments |
headers
A.J.Mechelynck | 1 May 2007 12:10
Picon

Re: feedkeys() allowed in sandbox

Bram Moolenaar wrote:
[...]
> Modelines are default off when you are root.  The mail filetype plugin
> also switches it off.
[...]

Are you sure? In a terminal logged-in as root, using vim 7.0.235:

	vim -u NONE -N
	:set ml? mls?
   modeline
   modelines=5

Modelines default off when 'compatible' is set, they default on (and 5) when 
'nocompatible' is set. Root login changes nothing to that AFAICT.

Best regards,
Tony.
--

-- 
     "You mean there really is an answer?"
     "Yes! But you're not going to like it!"
     "Oh do please tell us!"
     "You're really not going to like it!"
     "but we MUST know - tell us"
     "Alright, the answer is...."
     "yes..."
     "... is ..."
     "yes... come on!"
     "is 42!"
     		(Douglas Adams - The Hitchhiker's Guide to the Galaxy)
(Continue reading)

Permalink | Reply | 1 comment |
headers
Bram Moolenaar | 1 May 2007 13:32
Picon

Re: feedkeys() allowed in sandbox


Tony Mechelynck wrote:

> Bram Moolenaar wrote:
> [...]
> > Modelines are default off when you are root.  The mail filetype plugin
> > also switches it off.
> [...]
> 
> Are you sure? In a terminal logged-in as root, using vim 7.0.235:
> 
> 	vim -u NONE -N
> 	:set ml? mls?
>    modeline
>    modelines=5
> 
> Modelines default off when 'compatible' is set, they default on (and 5) when 
> 'nocompatible' is set. Root login changes nothing to that AFAICT.

Sorry, my mistake.  There is a recommendation that when working as root
you switch 'modeline' off, but it's not done automatically.

I do think that it's a good idea to make it automatic.

--

-- 
He who laughs last, thinks slowest.

 /// Bram Moolenaar -- Bram <at> Moolenaar.net -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
(Continue reading)

Permalink | Reply | 0 comments |
headers
Bram Moolenaar | 1 May 2007 13:39
Picon

patch 7.0.236


Patch 7.0.236
Problem:    Linux 2.4 uses sysinfo() with a mem_unit field, which is not
	    backwards compatible.
Solution:   Add an autoconf check for sysinfo.mem_unit.  Let mch_total_mem()
	    return Kbyte to avoid overflow.
Files:	    src/auto/configure, src/configure.in, src/config.h.in,
	    src/option.c, src/os_unix.c

*** ../vim-7.0.235/src/auto/configure	Tue Oct 17 11:50:45 2006
--- src/auto/configure	Thu Apr 26 16:44:07 2007
***************
*** 13685,13690 ****
--- 13685,13746 ----

  echo "$as_me:$LINENO: result: not usable" >&5
  echo "${ECHO_T}not usable" >&6
+ fi
+ rm -f conftest.err conftest.$ac_objext conftest.$ac_ext
+ 
+ echo "$as_me:$LINENO: checking for sysinfo.mem_unit" >&5
+ echo $ECHO_N "checking for sysinfo.mem_unit... $ECHO_C" >&6
+ cat >conftest.$ac_ext <<_ACEOF
+ /* confdefs.h.  */
+ _ACEOF
+ cat confdefs.h >>conftest.$ac_ext
+ cat >>conftest.$ac_ext <<_ACEOF
+ /* end confdefs.h.  */
+ #include <sys/types.h>
+ #include <sys/sysinfo.h>
(Continue reading)

Permalink | Reply | 2 comments |
headers
scott | 1 May 2007 18:14

Re: patch 7.0.236

i had to

   'rm src/auto/config.cache

before i could build with this one

sc
Permalink | Reply | 1 comment |
headers
A.J.Mechelynck | 1 May 2007 18:28
Picon

Re: patch 7.0.236

scott wrote:
> i had to
> 
>    'rm src/auto/config.cache
> 
> before i could build with this one
> 
> sc
> 

Yes, this is normal for a patch affecting the configure files:

	src/auto/configure
	src/configure.in
	src/config.h.in

Me too: the first make ended in an error, suggesting to remove 
auto/config.cache; then after "rm -vf auto/config.cache" the next make rebuilt 
everything.

Best regards,
Tony.
--

-- 
	THE LESSER-KNOWN PROGRAMMING LANGUAGES #2: RENE

Named after the famous French philosopher and mathematician Rene
DesCartes, RENE is a language used for artificial intelligence.  The
language is being developed at the Chicago Center of Machine Politics
and Programming under a grant from the Jane Byrne Victory Fund.  A
spokesman described the language as "Just as great as dis [sic] city of
(Continue reading)

Permalink | Reply | 0 comments |
headers
Bram Moolenaar | 1 May 2007 19:06
Picon

patch 7.0.237


Patch 7.0.237
Problem:    For root it is recommended to not use 'modeline', but in
	    not-compatible mode the default is on.
Solution:   Let 'modeline' default to off for root.
Files:	    runtime/doc/options.txt, src/option.c

*** ../vim-7.0.236/runtime/doc/options.txt	Tue Oct 10 18:43:50 2006
--- runtime/doc/options.txt	Tue May  1 13:29:08 2007
***************
*** 1,4 ****
! *options.txt*	For Vim version 7.0.  Last change: 2006 May 04

  
  		  VIM REFERENCE MANUAL	  by Bram Moolenaar
--- 1,4 ----
! *options.txt*	For Vim version 7.0.  Last change: 2007 May 01

  
  		  VIM REFERENCE MANUAL	  by Bram Moolenaar
***************
*** 528,534 ****
  ':' is removed.  Thus to include "\:" you have to specify "\\:".

  No other commands than "set" are supported, for security reasons (somebody
! might create a Trojan horse text file with modelines).

  Hint: If you would like to do something else than setting an option, you could
  define an autocommand that checks the file for a specific string.  For
--- 528,539 ----
(Continue reading)

Permalink | Reply | 1 comment |
headers
A.J.Mechelynck | 1 May 2007 19:52
Picon

Re: patch 7.0.237

Bram Moolenaar wrote:
> Patch 7.0.237
> Problem:    For root it is recommended to not use 'modeline', but in
> 	    not-compatible mode the default is on.
> Solution:   Let 'modeline' default to off for root.
> Files:	    runtime/doc/options.txt, src/option.c

Note:

	:set ml&vim
or
	:set nocompatible
	...
	:set ml&

still sets 'modeline' on, even when logged-in as root.

Best regards,
Tony.
--

-- 
If you want to understand your government, don't begin by reading the
Constitution.  It conveys precious little of the flavor of today's
statecraft.  Instead, read selected portions of the Washington
telephone directory containing listings for all the organizations with
titles beginning with the word "National".
		-- George Will
Permalink | Reply | 0 comments |
headers
Bram Moolenaar | 1 May 2007 22:06
Picon

patch 7.0.238


Patch 7.0.238
Problem:    Crash when ":match" pattern runs into 'maxmempattern'. (Yakov
	    Lerner)
Solution:   Don't free the regexp program of match_hl.
Files:	    src/screen.c

*** ../vim-7.0.237/src/screen.c	Tue Nov 28 16:16:03 2006
--- src/screen.c	Tue May  1 21:36:50 2007
***************
*** 6477,6485 ****
  	if (called_emsg)
  	{
  	    /* Error while handling regexp: stop using this regexp. */
! 	    vim_free(shl->rm.regprog);
  	    shl->rm.regprog = NULL;
! 	    no_hlsearch = TRUE;
  	    break;
  	}
  	if (nmatched == 0)
--- 6477,6491 ----
  	if (called_emsg)
  	{
  	    /* Error while handling regexp: stop using this regexp. */
! 	    if (shl == &search_hl)
! 	    {
! 		/* don't free the regprog in match_hl[], it's a copy */
! 		vim_free(shl->rm.regprog);
! 		no_hlsearch = TRUE;
! 	    }
(Continue reading)

Permalink | Reply | 0 comments |

Gmane