From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server-vie001.gnuweeb.org X-Spam-Level: X-Spam-Status: No, score=-0.2 required=5.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,PDS_OTHER_BAD_TLD, URIBL_DBL_BLOCKED_OPENDNS,URIBL_ZEN_BLOCKED_OPENDNS autolearn=no autolearn_force=no version=3.4.6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gnuweeb.org; s=new2025; t=1757673464; bh=Hi1M9tsRn2IdDIm9/W6ADHmRkjqazrxgV3Pw60TTkco=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type:Content-Transfer-Encoding:Message-ID:Date:From: Reply-To:Subject:To:Cc:In-Reply-To:References:Resent-Date: Resent-From:Resent-To:Resent-Cc:User-Agent:Content-Type: Content-Transfer-Encoding; b=iwn8x5uwCeBl/H9k0APX3tHdnE/i5QcD7fokZ5RXILxY6mP23iSzsT2EOi8GVCWKj 5+DPiuDDrwgUuohaEBIqvU12YVsEmVUZ6TeA6m/WLEIoHZW5jnb+Gk1q7rkPPoyzlT xfEpOPRK0SnyujS14d8T0DFOdkvzJ3PmbVeEt79LZ5H4tolpGndvbTBpVNWH+JUfLD ewdQJRHIfWEW2OisQV8KFzbi2lurhj+tTHKS1YEHFQ1XXWVbTdxS8g8EwMA1tszdXC wAUG1gtT0N0h+KheMzBAp9irf+jWbnGktKPEGMiHZc0wo6vHQD9b4erMukjhTwQ+Dw akZ2N0jdqFImA== Received: from mail-lj1-f171.google.com (mail-lj1-f171.google.com [209.85.208.171]) by server-vie001.gnuweeb.org (Postfix) with ESMTPSA id 4FF923127979 for ; Fri, 12 Sep 2025 10:37:44 +0000 (UTC) Received: by mail-lj1-f171.google.com with SMTP id 38308e7fff4ca-33c9efd65eeso16996251fa.3 for ; Fri, 12 Sep 2025 03:37:44 -0700 (PDT) X-Forwarded-Encrypted: i=1; AJvYcCWL5NAMQrXT7PSjl2NvVecTiv4Mmc5mOyXPK6L5OJTXHcWy9/5e87VYcg7jbh71AdBsFWCC@vger.gnuweeb.org X-Gm-Message-State: AOJu0YwF4PrhSQr6W+KuJQ7oYbqVeLKHlSfAHLuVPXyqcNEUqjqq9mFT uifLRAflHjvtf0WtVM+R9IerFvUsdDrT9ZboTaWwE615mPrDz0u9b4GG53TED2H/H69RHmnld1t r1raSuPguV2mwF29QefKMU8DXHmvShWs= X-Google-Smtp-Source: AGHT+IGDmnhoGdBJqLRnY/LK3iNuWIFTlhxUd7ZC6TTe99CtMJ33EWaIxwees75wY07Pey5P5zyOz00CJgD8d/fpgo8= X-Received: by 2002:a05:651c:4392:20b0:336:7747:708 with SMTP id 38308e7fff4ca-3513a044653mr5869651fa.2.1757673463662; Fri, 12 Sep 2025 03:37:43 -0700 (PDT) MIME-Version: 1.0 References: <20250910104326.580778-1-reyuki@gnuweeb.org> <20250910104326.580778-3-reyuki@gnuweeb.org> In-Reply-To: From: Ahmad Gani Date: Fri, 12 Sep 2025 17:37:05 +0700 X-Gmail-Original-Message-ID: X-Gm-Features: Ac12FXyL9LEqno31INjk1Ev2L1B0l_Ys4WCNSTb2-EZTn77a-zRf3O5YGbvh7mc Message-ID: Subject: Re: [PATCH gwproxy v10 2/2] gwproxy: refactor code base to add experimental raw DNS backend To: Alviro Iskandar Setiawan Cc: Ammar Faizi , "GNU/Weeb Mailing List" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable List-Id: On Thu, Sep 11, 2025 at 5:11=E2=80=AFPM Alviro Iskandar Setiawan wrote: > On Wed, Sep 10, 2025 at 5:44=E2=80=AFPM Ahmad Gani wrote: > > + cfg =3D &w->ctx->cfg; > > + resolv =3D &w->dns_resolver; > > + p1 =3D realloc(resolv->stack.arr, cfg->sess_map_cap * sizeof(*r= esolv->stack.arr)); > > + if (!p1) > > + return; > > + resolv->stack.arr =3D p1; > > + > > + p2 =3D realloc(resolv->sess_map, cfg->sess_map_cap * sizeof(*re= solv->sess_map)); > > + if (!p2) > > + return; > > + resolv->sess_map =3D p2; > > This shrink operation looks very *dangerous* to me. Think, what > happens if "p2 =3D realloc()" fails? > > You update "->stack.arr =3D p1;" but given "->stack.top >=3D > ->sess_map_cap", now ->stack.top is pointing to an index beyond the > array capacity because you have just successfully shrunk the array > capacity down to ->sess_map_cap. Next time, a client with SOCKS5 > hostname comes in, you call pop_txid(), and you will explode > ->stack.arr[--resolv->stack.top] ** OVERFLOW ** > because ->stack.top is not updated. That kind of pattern is only safe > for array expansion, not for this shrink op definitely. You're allowed > to over allocate the size, but not under allocate. That does sound dangerous. Even when shrinking, realloc can still fail if it can't allocate a smaller block. > > + memset(p2, 0, cfg->sess_map_cap * sizeof(*resolv->sess_map)); > > + > > + i =3D cfg->sess_map_cap; > > + resolv->stack.top =3D i--; > > + for (; i <=3D 0; i--) > > + p1[i] =3D i; > > + > > + resolv->sess_map_cap =3D cfg->sess_map_cap; > > Meanwhile, ->stack.top *is* only updated when p2 is successfully > shrunk, what if *only the p1 succeeds*? In this case, would it be better to allocate both sess_map and stack.arr in a single block, so we only call realloc once? > My hunch is that you may still need some basic training on C > programming. I'd also be happy to to go through a basic training session on C too! :3 > Anyway, this patch is doing too many things. It's not something you > should do in a single commit. Please split it into smaller, more > manageable pieces. Alright, I will try to split the patches. -- Ahmad Gani