type of request : enhancement 

version: openssl-1.0.0e
version: openssl-0.9.8g

Hello,

Checking bogus EC public key may lead to a crash if the public key point
do not have its Z_is_one member set.

In the case this bit is not set, ec_GF2m_simple_is_on_curve goto error
handler which make a BN_CTX_end. In this case BN_CTX_start has not been
called.

Best regards
Emmanuel

Breakpoint 2, vxExcHookWrapper (vec=768, pESF=0x3ae6b48 "\003®l\b",
    pRegs=0x3ae6b60)
    at /home/azencot/bnt/target.4/config/comps/src/usrWdbCore.c:386
386         (*vxExcHook)(context, vec, pESF, pRegs);
(gdb) bt
#0  vxExcHookWrapper (vec=768, pESF=0x3ae6b48 "\003®l\b", pRegs=0x3ae6b60)
    at /home/azencot/bnt/target.4/config/comps/src/usrWdbCore.c:386
#1  0x0013f020 in excExcHandle ()
#2  0x00000318 in ?? ()
#3  0x00000300 in ?? ()
#4  0x00359198 in v_secd_mempart_pool ()
#5  0x000c2e4c in ec_GF2m_simple_is_on_curve (group=0x359360, point=0x359460,
    ctx=0x359428) at ec2_smpl.c:852
#6  0x000b8838 in EC_POINT_is_on_curve (group=0x0, point=0x3ae6b48,
    ctx=0x3ae6b60) at ec_lib.c:1055
#7  0x000c127c in EC_KEY_check_key (eckey=0x359568) at ec_key.c:320

....

(gdb) show endian
The target is assumed to be big endian
(gdb) p pRegs
$5 = (WDB_IU_REGS *) 0x3ae6b60
(gdb) p /x *pRegs
$7 = {gpr = {0xc2e4c, 0x3ae6c08, 0x0, 0x35943c, 0x359460, 0x359428, 0x5,
    0xfffffff8, 0x0, 0xffffffff, 0x0, 0xfffffffc, 0x40000048, 0x0, 0x0, 0x0,
    0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xffffffff, 0xc3a24, 0x0, 0xc39f0, 0x0,
    0x359460, 0x359360, 0x0, 0x359428, 0x359428}, msr = 0xb030, lr = 0xa93bc,
  ctr = 0x0, pc = 0xa963c, cr = 0x20000028, xer = 0x20000000, pad = 0x3ae6c18}
(gdb) x /10i (pRegs->pc-20)
0xa9628 <BN_STACK_pop>: l       r9,4(r3)
0xa962c <BN_STACK_pop+4>:       l       r10,0(r3)
0xa9630 <BN_STACK_pop+8>:       cal     r9,-1(r9)
0xa9634 <BN_STACK_pop+12>:      rlinm   r11,r9,2,0,29

0xa9638 <BN_STACK_pop+16>:      st      r9,4(r3) <===

0xa963c <BN_STACK_pop+20>:      lx      r3,r11,r10
0xa9640 <BN_STACK_pop+24>:      br
0xa9644 <bn_sub_part_words>:    stu     r1,-32(r1)
0xa9648 <bn_sub_part_words+4>:  mflr    r0
0xa964c <bn_sub_part_words+8>:  st      r27,12(r1)

(gdb) p /x (pRegs->gpr[3])
$8 = 0x35943c
(gdb) x /x (pRegs->gpr[3])
0x35943c <v_secd_mempart_pool+520672>:  0x00000000
(gdb) x /x (pRegs->gpr[3]+4)
0x359440 <v_secd_mempart_pool+520676>:  0xffffffff
(gdb) p /x (pRegs->gpr[9])
$9 = 0xffffffff
(gdb) p /x (pRegs->gpr[10])
$10 = 0x0
(gdb) l *(pRegs->pc)
0xa963c is in BN_STACK_pop (bn_ctx.c:353).
348             return 1;
349             }
350
351     static unsigned int BN_STACK_pop(BN_STACK *st)
352             {
353 ===>        return st->indexes[--(st->depth)];
354             }
(gdb) l *(pRegs->lr)
0xa93bc is in BN_CTX_end (bn_ctx.c:279).
270     void BN_CTX_end(BN_CTX *ctx)
271             {
272             CTXDBG_ENTRY("BN_CTX_end", ctx);
273             if(ctx->err_stack)
274                     ctx->err_stack--;
275             else
276                     {
277 ===>                unsigned int fp = BN_STACK_pop(&ctx->stack);
278                     /* Does this stack frame have anything to release? */
279                     if(fp < ctx->used)
280                             BN_POOL_release(&ctx->pool, ctx->used - fp);
281                     ctx->used = fp;
282                     /* Unjam "too_many" in case "get" had failed */
283                     ctx->too_many = 0;
(gdb) l ec2_smpl.c:809
809     int ec_GF2m_simple_is_on_curve(const EC_GROUP *group, const EC_POINT 
*point, BN_CTX *ctx)
810             {
811             int ret = -1;
812             BN_CTX *new_ctx = NULL;
813             BIGNUM *lh, *y2;
814             int (*field_mul)(const EC_GROUP *, BIGNUM *, const BIGNUM *, 
const BIGNUM *, BN_CTX *);
815             int (*field_sqr)(const EC_GROUP *, BIGNUM *, const BIGNUM *, 
BN_CTX *);
816
817             if (EC_POINT_is_at_infinity(group, point))
818                     return 1;
819
820             field_mul = group->meth->field_mul;
821             field_sqr = group->meth->field_sqr;
822
823             /* only support affine coordinates */
824 -           if (!point->Z_is_one) goto err;
    +           if (!point->Z_is_one) {
      +              ECerr(EC_F_EC_GF2M_SIMPLE_IS_ON_CURVE, 
EC_R_INVALID_ARGUMENT);
      +              return -1;
      +           }
825
826             if (ctx == NULL)
827                     {
828                     ctx = new_ctx = BN_CTX_new();
829                     if (ctx == NULL)
830                             return -1;
831                     }
832
833             BN_CTX_start(ctx);
834             y2 = BN_CTX_get(ctx);
835             lh = BN_CTX_get(ctx);
836             if (lh == NULL) goto err;
837
838             /* We have a curve defined by a Weierstrass equation
839              *      y^2 + x*y = x^3 + a*x^2 + b.
840              *  <=> x^3 + a*x^2 + x*y + b + y^2 = 0
841              *  <=> ((x + a) * x + y ) * x + b + y^2 = 0
842              */
843             if (!BN_GF2m_add(lh, &point->X, &group->a)) goto err;
844             if (!field_mul(group, lh, lh, &point->X, ctx)) goto err;
845             if (!BN_GF2m_add(lh, lh, &point->Y)) goto err;
846             if (!field_mul(group, lh, lh, &point->X, ctx)) goto err;
847             if (!BN_GF2m_add(lh, lh, &group->b)) goto err;
848             if (!field_sqr(group, y2, &point->Y, ctx)) goto err;
849             if (!BN_GF2m_add(lh, lh, y2)) goto err;
850             ret = BN_is_zero(lh);
851ÿÿÿÿÿ err:
852 ===>ÿÿÿÿÿÿÿ if (ctx) BN_CTX_end(ctx);
853ÿÿÿÿÿÿÿÿÿÿÿÿ if (new_ctx) BN_CTX_free(new_ctx);
854ÿÿÿÿÿÿÿÿÿÿÿÿ return ret;
855ÿÿÿÿÿÿÿÿÿÿÿÿ }
(gdb) p ctx
$11 = (BN_CTX *) 0x359428
(gdb) p /x *ctx
$13 = {pool = {head = 0x0, current = 0x0, tail = 0x0, used = 0x0, size = 0x0},
ÿ stack = {indexes = 0x0, depth = 0xffffffff, size = 0x0}, used = 0x0,
ÿ err_stack = 0x0, too_many = 0x0}
(gdb) p /x point
$25 = 0x359460
(gdb) p /x *point
$26 = {meth = 0x25473c, X = {d = 0x359128, top = 0xd, dmax = 0xd, neg = 0x0,
ÿÿÿ flags = 0x0}, Y = {d = 0x3590e8, top = 0xd, dmax = 0xd, neg = 0x0,
ÿÿÿ flags = 0x0}, Z = {d = 0x3592d0, top = 0x1, dmax = 0x1, neg = 0x0,
ÿÿÿ flags = 0x0}, Z_is_one = 0x0}


type of request : enhancement

version: openssl-1.0.0e
version: openssl-0.9.8g

Hello,

Checking bogus EC public key may lead to a crash if the public key point
do not have its Z_is_one member set.

In the case this bit is not set, ec_GF2m_simple_is_on_curve goto error
handler which make a BN_CTX_end. In this case BN_CTX_start has not been
called.

Best regards
Emmanuel

Breakpoint 2, vxExcHookWrapper (vec=768, pESF=0x3ae6b48 "\003?l\b",
    pRegs=0x3ae6b60)
    at /home/azencot/bnt/target.4/config/comps/src/usrWdbCore.c:386
386         (*vxExcHook)(context, vec, pESF, pRegs);
(gdb) bt
#0  vxExcHookWrapper (vec=768, pESF=0x3ae6b48 "\003?l\b", pRegs=0x3ae6b60)
    at /home/azencot/bnt/target.4/config/comps/src/usrWdbCore.c:386
#1  0x0013f020 in excExcHandle ()
#2  0x00000318 in ?? ()
#3  0x00000300 in ?? ()
#4  0x00359198 in v_secd_mempart_pool ()
#5  0x000c2e4c in ec_GF2m_simple_is_on_curve (group=0x359360, point=0x359460,
    ctx=0x359428) at ec2_smpl.c:852
#6  0x000b8838 in EC_POINT_is_on_curve (group=0x0, point=0x3ae6b48,
    ctx=0x3ae6b60) at ec_lib.c:1055
#7  0x000c127c in EC_KEY_check_key (eckey=0x359568) at ec_key.c:320

....

(gdb) show endian
The target is assumed to be big endian
(gdb) p pRegs
$5 = (WDB_IU_REGS *) 0x3ae6b60
(gdb) p /x *pRegs
$7 = {gpr = {0xc2e4c, 0x3ae6c08, 0x0, 0x35943c, 0x359460, 0x359428, 0x5,
    0xfffffff8, 0x0, 0xffffffff, 0x0, 0xfffffffc, 0x40000048, 0x0, 0x0, 0x0,
    0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xffffffff, 0xc3a24, 0x0, 0xc39f0, 0x0,
    0x359460, 0x359360, 0x0, 0x359428, 0x359428}, msr = 0xb030, lr = 0xa93bc,
  ctr = 0x0, pc = 0xa963c, cr = 0x20000028, xer = 0x20000000, pad = 0x3ae6c18}
(gdb) x /10i (pRegs->pc-20)
0xa9628 <BN_STACK_pop>: l       r9,4(r3)
0xa962c <BN_STACK_pop+4>:       l       r10,0(r3)
0xa9630 <BN_STACK_pop+8>:       cal     r9,-1(r9)
0xa9634 <BN_STACK_pop+12>:      rlinm   r11,r9,2,0,29

0xa9638 <BN_STACK_pop+16>:      st      r9,4(r3) <===

0xa963c <BN_STACK_pop+20>:      lx      r3,r11,r10
0xa9640 <BN_STACK_pop+24>:      br
0xa9644 <bn_sub_part_words>:    stu     r1,-32(r1)
0xa9648 <bn_sub_part_words+4>:  mflr    r0
0xa964c <bn_sub_part_words+8>:  st      r27,12(r1)

(gdb) p /x (pRegs->gpr[3])
$8 = 0x35943c
(gdb) x /x (pRegs->gpr[3])
0x35943c <v_secd_mempart_pool+520672>:  0x00000000
(gdb) x /x (pRegs->gpr[3]+4)
0x359440 <v_secd_mempart_pool+520676>:  0xffffffff
(gdb) p /x (pRegs->gpr[9])
$9 = 0xffffffff
(gdb) p /x (pRegs->gpr[10])
$10 = 0x0
(gdb) l *(pRegs->pc)
0xa963c is in BN_STACK_pop (bn_ctx.c:353).
348             return 1;
349             }
350
351     static unsigned int BN_STACK_pop(BN_STACK *st)
352             {
353 ===>        return st->indexes[--(st->depth)];
354             }
(gdb) l *(pRegs->lr)
0xa93bc is in BN_CTX_end (bn_ctx.c:279).
270     void BN_CTX_end(BN_CTX *ctx)
271             {
272             CTXDBG_ENTRY("BN_CTX_end", ctx);
273             if(ctx->err_stack)
274                     ctx->err_stack--;
275             else
276                     {
277 ===>                unsigned int fp = BN_STACK_pop(&ctx->stack);
278                     /* Does this stack frame have anything to release? */
279                     if(fp < ctx->used)
280                             BN_POOL_release(&ctx->pool, ctx->used - fp);
281                     ctx->used = fp;
282                     /* Unjam "too_many" in case "get" had failed */
283                     ctx->too_many = 0;
(gdb) l ec2_smpl.c:809
809     int ec_GF2m_simple_is_on_curve(const EC_GROUP *group, const EC_POINT *point, BN_CTX *ctx)
810             {
811             int ret = -1;
812             BN_CTX *new_ctx = NULL;
813             BIGNUM *lh, *y2;
814             int (*field_mul)(const EC_GROUP *, BIGNUM *, const BIGNUM *, const BIGNUM *, BN_CTX *);
815             int (*field_sqr)(const EC_GROUP *, BIGNUM *, const BIGNUM *, BN_CTX *);
816
817             if (EC_POINT_is_at_infinity(group, point))
818                     return 1;
819
820             field_mul = group->meth->field_mul;
821             field_sqr = group->meth->field_sqr;
822
823             /* only support affine coordinates */
824 -           if (!point->Z_is_one) goto err;
    +           if (!point->Z_is_one) {
      +              ECerr(EC_F_EC_GF2M_SIMPLE_IS_ON_CURVE, EC_R_INVALID_ARGUMENT);
      +              return -1;
      +           }
825
826             if (ctx == NULL)
827                     {
828                     ctx = new_ctx = BN_CTX_new();
829                     if (ctx == NULL)
830                             return -1;
831                     }
832
833             BN_CTX_start(ctx);
834             y2 = BN_CTX_get(ctx);
835             lh = BN_CTX_get(ctx);
836             if (lh == NULL) goto err;
837
838             /* We have a curve defined by a Weierstrass equation
839              *      y^2 + x*y = x^3 + a*x^2 + b.
840              *  <=> x^3 + a*x^2 + x*y + b + y^2 = 0
841              *  <=> ((x + a) * x + y ) * x + b + y^2 = 0
842              */
843             if (!BN_GF2m_add(lh, &point->X, &group->a)) goto err;
844             if (!field_mul(group, lh, lh, &point->X, ctx)) goto err;
845             if (!BN_GF2m_add(lh, lh, &point->Y)) goto err;
846             if (!field_mul(group, lh, lh, &point->X, ctx)) goto err;
847             if (!BN_GF2m_add(lh, lh, &group->b)) goto err;
848             if (!field_sqr(group, y2, &point->Y, ctx)) goto err;
849             if (!BN_GF2m_add(lh, lh, y2)) goto err;
850             ret = BN_is_zero(lh);
851      err:
852 ===>        if (ctx) BN_CTX_end(ctx);
853             if (new_ctx) BN_CTX_free(new_ctx);
854             return ret;
855             }
(gdb) p ctx
$11 = (BN_CTX *) 0x359428
(gdb) p /x *ctx
$13 = {pool = {head = 0x0, current = 0x0, tail = 0x0, used = 0x0, size = 0x0},
  stack = {indexes = 0x0, depth = 0xffffffff, size = 0x0}, used = 0x0,
  err_stack = 0x0, too_many = 0x0}
(gdb) p /x point
$25 = 0x359460
(gdb) p /x *point
$26 = {meth = 0x25473c, X = {d = 0x359128, top = 0xd, dmax = 0xd, neg = 0x0,
    flags = 0x0}, Y = {d = 0x3590e8, top = 0xd, dmax = 0xd, neg = 0x0,
    flags = 0x0}, Z = {d = 0x3592d0, top = 0x1, dmax = 0x1, neg = 0x0,
    flags = 0x0}, Z_is_one = 0x0}

Reply via email to