[vim/vim] Vim 8.0.5 crashes if text begining of text mimic encrypted text header (#1096)

[igor2x]

Hi,
I installed Vim 8.0.5 on Ubuntu 16.04 using PPA following the following instructions: http://tipsonubuntu.com/2016/09/13/vim-8-0-released-install-ubuntu-16-04/

Just for fun I tried to see what happens if file begins with the same text as Vim encrypted file. Interesting to find out Vim crashes.

To reproduce a problem:
1. I opened empty file from bash shell: vim test.txt
2. Typed letter I to enter insert mode.
3. Typed into file the following text: VimCrypt~03!abc
4. Save and quit: :wq
5. Open file: vim test.txt
6. Encryption key appears:
a) If just pressing Enter (so without password) then file opens normally without a problem.
b) If typed in any dummy text for password like: test
and Vim crashes with error:

Vim: Caught deadly signal SEGV
vi: malloc.c:3723: _int_malloc: Assertion `(unsigned long) (size) >= (unsigned long) (nb)' failed
Vim: Finished
Vim: Double signal, exiting

In my humble opinion program should never crash, but should return some error or similar.

The same crash if in step 3 using text:
VimCrypt~02!abc

But without crash if in step 3 using text:
VimCrypt~01!abc

I attached file with context from command from bash shell: vim --version

P.S. Because Vim crashes with "VimCrypt~02!abc" text this crash is probably already in vim 7.3+ when blowfish was first released in Vim. I haven't tested in any other version beside vim 8.0.5
vim_version.txt

Regards

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub

[Christian Brabandt]

This patch fixes it.

diff --git a/src/fileio.c b/src/fileio.c
index ea1f338..193e4fd 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -3011,6 +3011,9 @@ check_for_cryptkey(

            /* Remove cryptmethod specific header from the text. */
            header_len = crypt_get_header_len(method);
+           if (*sizep <= header_len)
+               /* buffer can't be encrypted */
+               return NULL;
            *filesizep += header_len;
            *sizep -= header_len;
            mch_memmove(ptr, ptr + header_len, (size_t)*sizep);

[Bram Moolenaar]

Christian Brabandt wrote:

> This patch fixes it.
> ```patch

> diff --git a/src/fileio.c b/src/fileio.c
> index ea1f338..193e4fd 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -3011,6 +3011,9 @@ check_for_cryptkey(
>
> /* Remove cryptmethod specific header from the text. */
> header_len = crypt_get_header_len(method);
> + if (*sizep <= header_len)
> + /* buffer can't be encrypted */
> + return NULL;
> *filesizep += header_len;
> *sizep -= header_len;
> mch_memmove(ptr, ptr + header_len, (size_t)*sizep);

> ```

Thanks. Would be good to have a test. Should be possible by writing an
encrypted file and then opening it in binary mode.

--
You were lucky. We lived for three months in a brown paper bag in a
septic tank. We used to have to get up at six o'clock in the morning,
clean the bag, eat a crust of stale bread, go to work down mill for
fourteen hours a day week in-week out. When we got home, our Dad
would thrash us to sleep with his belt!

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[vim-dev ML]

Hi Bram!

On Do, 22 Sep 2016, Bram Moolenaar wrote:

>
> Christian Brabandt wrote:
>
> > This patch fixes it.
> > ```patch
> > diff --git a/src/fileio.c b/src/fileio.c
> > index ea1f338..193e4fd 100644
> > --- a/src/fileio.c
> > +++ b/src/fileio.c
> > @@ -3011,6 +3011,9 @@ check_for_cryptkey(
> >
> > /* Remove cryptmethod specific header from the text. */
> > header_len = crypt_get_header_len(method);
> > + if (*sizep <= header_len)
> > + /* buffer can't be encrypted */
> > + return NULL;
> > *filesizep += header_len;
> > *sizep -= header_len;
> > mch_memmove(ptr, ptr + header_len, (size_t)*sizep);
> > ```
>
> Thanks. Would be good to have a test. Should be possible by writing an
> encrypted file and then opening it in binary mode.

Updated patch attached.

Best,
Christian
--
Vor der Revolution war alles Bestreben, nachher verwandelte sich
alles in Forderung.
-- Goethe, Maximen und Reflektionen, Nr. 562

[Christian Brabandt]

On So, 25 Sep 2016, vim-dev ML wrote:

> Hi Bram!
>
> On Do, 22 Sep 2016, Bram Moolenaar wrote:
>
> >
> > Christian Brabandt wrote:
> >
> > > This patch fixes it.
> > > ```patch
> > > diff --git a/src/fileio.c b/src/fileio.c
> > > index ea1f338..193e4fd 100644
> > > --- a/src/fileio.c
> > > +++ b/src/fileio.c
> > > @@ -3011,6 +3011,9 @@ check_for_cryptkey(
> > >
> > > /* Remove cryptmethod specific header from the text. */
> > > header_len = crypt_get_header_len(method);
> > > + if (*sizep <= header_len)
> > > + /* buffer can't be encrypted */
> > > + return NULL;
> > > *filesizep += header_len;
> > > *sizep -= header_len;
> > > mch_memmove(ptr, ptr + header_len, (size_t)*sizep);
> > > ```
> >
> > Thanks. Would be good to have a test. Should be possible by writing an
> > encrypted file and then opening it in binary mode.
>
> Updated patch attached.

Sorry, the patch did not survive github. This time really attached to
the vim-dev list.

Best,
Christian
--
Wenn man einen Tausendfüßler erklären lassen würde, nach welchem
Schema er eigentlich gehe, so wird dieser auf der Stelle unfähig sein,
auch nur einen einzelnen Schritt zu machen.
-- Josef Mittendorfer

[Christian Brabandt]

closed by 8.0.010

—
You are receiving this because you commented.

[Christian Brabandt]

Closed #1096.

—
You are receiving this because you commented.

[Bram Moolenaar]

Christian Brabandt wrote:

> > Hi Bram!
> >
> > On Do, 22 Sep 2016, Bram Moolenaar wrote:
> >
> > >
> > > Christian Brabandt wrote:
> > >
> > > > This patch fixes it.
> > > > ```patch
> > > > diff --git a/src/fileio.c b/src/fileio.c
> > > > index ea1f338..193e4fd 100644
> > > > --- a/src/fileio.c
> > > > +++ b/src/fileio.c
> > > > @@ -3011,6 +3011,9 @@ check_for_cryptkey(
> > > >
> > > > /* Remove cryptmethod specific header from the text. */
> > > > header_len = crypt_get_header_len(method);
> > > > + if (*sizep <= header_len)
> > > > + /* buffer can't be encrypted */
> > > > + return NULL;
> > > > *filesizep += header_len;
> > > > *sizep -= header_len;
> > > > mch_memmove(ptr, ptr + header_len, (size_t)*sizep);
> > > > ```
> > >
> > > Thanks. Would be good to have a test. Should be possible by writing an
> > > encrypted file and then opening it in binary mode.
> >
> > Updated patch attached.
>
> Sorry, the patch did not survive github. This time really attached to
> the vim-dev list.

I had just sent out a patch with a similar test. It was based on the
original problem description. Sorry for being impatient!


--
FATHER: Who are you?
PRINCE: I'm ... your son ...
FATHER: Not you.
LAUNCELOT: I'm ... er ... Sir Launcelot, sir.
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

[igor2x]

I have updated Vim to 8.0.13 on Ubuntu 16.04, retested all three tests and now Vim works fine without crashes. Thanks a lot for fixing this.

—
You are receiving this because you commented.