linux-kernel - Re: [GIT] More security subsystem fixes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 25 Jan 2012 14:26:29 +0200
From:	"Kasatkin, Dmitry" <dmitry.kasatkin@...el.com>
To:	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:	jmorris@...ei.org, linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org
Subject: Re: [GIT] More security subsystem fixes

On Thu, Jan 19, 2012 at 4:21 PM, Tetsuo Handa
<penguin-kernel@...ove.sakura.ne.jp> wrote:
> Tetsuo Handa wrote:
>> James Morris wrote:
>> >       MPILIB: Add a missing ENOMEM check
>> Maybe one more (shown below) with some random comments.
>
> Looked a bit more.
>
>
>
> In lib/mpi/mpih-div.c:
>
> mpi_limb_t
> mpihelp_divrem(mpi_ptr_t qp, mpi_size_t qextra_limbs,
>               mpi_ptr_t np, mpi_size_t nsize, mpi_ptr_t dp, mpi_size_t dsize)
> {
>        mpi_limb_t most_significant_q_limb = 0;
>
>        switch (dsize) {
>        case 0:
>                /* We are asked to divide by zero, so go ahead and do it!  (To make
>                   the compiler not remove this statement, return the value.)  */
>                return 1 / dsize;
>
> What's this? Division by 0 in the kernel is no good.
>
>
>
> In lib/mpi/mpicoder.c:
>
> MPI do_encode_md(const void *sha_buffer, unsigned nbits)
> {
> (...snipped...)
>        n = 0;
>        frame[n++] = 0;
>        frame[n++] = 1;         /* block type */
>        i = nframe - SHA1_DIGEST_LENGTH - asnlen - 3;
>
>        if (i <= 1) {
>                pr_info("MPI: message digest encoding failed\n");
>                kfree(frame);
>                return a;
>        }
>
>        memset(frame + n, 0xff, i);
>        n += i;
>        frame[n++] = 0;
>        memcpy(frame + n, &asn, asnlen);
>        n += asnlen;
>        memcpy(frame + n, sha_buffer, SHA1_DIGEST_LENGTH);
>        n += SHA1_DIGEST_LENGTH;
>
>        i = nframe;
>        fr_pt = frame;
>
>        if (n != nframe) {
>
> What's this? i = nframe - SHA1_DIGEST_LENGTH - asnlen - 3; equals
> nframe = i + SHA1_DIGEST_LENGTH + asnlen + 3; and this should be always true.

In fact n always equals to nframe, and this check is always false...


> Also, i = nframe; seems to make no sense because i is no longer used.
>
>                printk
>                    ("MPI: message digest encoding failed, frame length is wrong\n");
>                kfree(frame);
>                return a;
>        }
>
>        a = mpi_alloc((nframe + BYTES_PER_MPI_LIMB - 1) / BYTES_PER_MPI_LIMB);
>
> Missing a != NULL check.
>
>        mpi_set_buffer(a, frame, nframe, 0);
>        kfree(frame);
>
>        return a;
> }
>
>
>
> In lib/mpi/mpi-pow.c:
>
> int mpi_powm(MPI res, MPI base, MPI exp, MPI mod)
> {
> (...snipped...)
>        if (!msize)
>                msize = 1 / msize;      /* provoke a signal */
> (...snipped...)
> }
>
> Division by 0.
>
>
>
> In lib/mpi/mpi-div.c:
>
> int mpi_tdiv_q_2exp(MPI w, MPI u, unsigned count)
> {
> (...snipped...)
>        usize = u->nlimbs;
>        limb_cnt = count / BITS_PER_MPI_LIMB;
>        wsize = usize - limb_cnt;
>        if (limb_cnt >= usize)
>                w->nlimbs = 0;
>        else {
>                mpi_ptr_t wp;
>                mpi_ptr_t up;
>
>                if (RESIZE_IF_NEEDED(w, wsize) < 0)
>                        return -ENOMEM;
>                wp = w->d;
>                up = u->d;
>
>                count %= BITS_PER_MPI_LIMB;
>                if (count) {
>                        mpihelp_rshift(wp, up + limb_cnt, wsize, count);
>                        wsize -= !wp[wsize - 1];
> (...snipped...)
> }
>
> Is wsize > 0 guaranteed?
>
Should be, because of the check:
        if (limb_cnt >= usize)
                w->nlimbs = 0;
       else...


>
>
> Fixes (if any) will be needed to RHEL6's 2.6.32-220.2.1.el6 kernel as well.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/