lkml.org 
[lkml]   [2021]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH 5.10 68/77] sctp: add vtag check in sctp_sf_violation
    From
    Date
    Hello!

    It seems the patch may lead to NULL pointer dereference.


    1. sctp_sf_violation_chunk() calls sctp_sf_violation() with asoc arg
    equal to NULL.

    static enum sctp_disposition sctp_sf_violation_chunk(
    ...
    {
    ...
    if (!asoc)
    return sctp_sf_violation(net, ep, asoc, type, arg, commands);
    ...

    2. Newly added code of sctp_sf_violation() calls to sctp_vtag_verify()
    with asoc arg equal to NULL.

    enum sctp_disposition sctp_sf_violation(struct net *net,
    ...
    {
    struct sctp_chunk *chunk = arg;

    if (!sctp_vtag_verify(chunk, asoc))
    return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
    ...

    3. sctp_vtag_verify() dereferences asoc without any check.

    /* Check VTAG of the packet matches the sender's own tag. */
    static inline int
    sctp_vtag_verify(const struct sctp_chunk *chunk,
    const struct sctp_association *asoc)
    {
    /* RFC 2960 Sec 8.5 When receiving an SCTP packet, the endpoint
    * MUST ensure that the value in the Verification Tag field of
    * the received SCTP packet matches its own Tag. If the received
    * Verification Tag value does not match the receiver's own
    * tag value, the receiver shall silently discard the packet...
    */
    if (ntohl(chunk->sctp_hdr->vtag) != asoc->c.my_vtag)
    return 0;


    Found by Linux Verification Center (linuxtesting.org) with SVACE tool.

    --
    Best regards,
    Alexey Khoroshilov
    Linux Verification Center, ISPRAS


    On 01.11.2021 12:17, Greg Kroah-Hartman wrote:
    > From: Xin Long <lucien.xin@gmail.com>
    >
    > [ Upstream commit aa0f697e45286a6b5f0ceca9418acf54b9099d99 ]
    >
    > sctp_sf_violation() is called when processing HEARTBEAT_ACK chunk
    > in cookie_wait state, and some other places are also using it.
    >
    > The vtag in the chunk's sctphdr should be verified, otherwise, as
    > later in chunk length check, it may send abort with the existent
    > asoc's vtag, which can be exploited by one to cook a malicious
    > chunk to terminate a SCTP asoc.
    >
    > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
    > Signed-off-by: Xin Long <lucien.xin@gmail.com>
    > Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
    > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
    > Signed-off-by: Sasha Levin <sashal@kernel.org>
    > ---
    > net/sctp/sm_statefuns.c | 3 +++
    > 1 file changed, 3 insertions(+)
    >
    > diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
    > index 0cfbf6046bf8..324c0222d9e6 100644
    > --- a/net/sctp/sm_statefuns.c
    > +++ b/net/sctp/sm_statefuns.c
    > @@ -4549,6 +4549,9 @@ enum sctp_disposition sctp_sf_violation(struct net *net,
    > {
    > struct sctp_chunk *chunk = arg;
    >
    > + if (!sctp_vtag_verify(chunk, asoc))
    > + return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
    > +
    > /* Make sure that the chunk has a valid length. */
    > if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_chunkhdr)))
    > return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
    >

    \
     
     \ /
      Last update: 2021-11-02 15:22    [W:3.034 / U:0.556 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site