Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 10 May 2022 13:28:38 +0200
From:   Jan Kara <jack@suse.cz>
To:     butt3rflyh4ck <butterflyhuangxx@gmail.com>
Cc:     jack@suse.com, LKML <linux-kernel@vger.kernel.org>,
        linux-fsdevel@vger.kernel.org
Subject: Re: A slab-out-of-bounds Write when invoke udf_write_fi via ioctl
Message-ID: <20220510112838.zfx2sxyoh4ewmjxr@quack3.lan>
References: <CAFcO6XNpj6e+OGba30usr_miODhbhmqk7vh3ymU+NLoe2HBqFA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAFcO6XNpj6e+OGba30usr_miODhbhmqk7vh3ymU+NLoe2HBqFA@mail.gmail.com>
Precedence: bulk

On Tue 10-05-22 15:14:42, butt3rflyh4ck wrote:
> Hi, if mounts a malicious udf image,  there is a slab out of bounds
> write  bug when a user invokes udf_write_fi via ioctl.
> I have reproduced it in the latest kernel.
> 
> ##smaple analyse
> the function call chains:
> do_sys_open
>     --->do_sys_openat2
>        --->do_filp_open
>           --->path_openat
>               --->open_last_lookups
>                  --->lookup_open
>                      --->udf_add_nondir
>                         --->udf_add_entry
> 
> There would traverse to get a `fi` in the function udf_add_entry.
> ```
> if (dinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {   [1]
>          block = dinfo->i_location.logicalBlockNum;
>          fi = (struct fileIdentDesc *)
>            (dinfo->i_data + fibh->soffset -
>              udf_ext0_offset(dir) +
>               dinfo->i_lenEAttr);
>              [2]
> } else {
>           block = eloc.logicalBlockNum +
>               ((elen - 1) >>
>             dir->i_sb->s_blocksize_bits);
>             fi = (struct fileIdentDesc *)
>                  (fibh->sbh->b_data + fibh->soffset);
> }
> ```
> [1] if dinfo->i_alloc_type is ICBTAG_FLAG_AD_IN_ICB, [2] it  would
> calculate an offset as `fi`,
> through the debugger, the `fi` is as below:
> ```
>  p/x *(struct fileIdentDesc*)fi
> $24 = {
>   descTag = {
>     tagIdent = 0x2f70,
>     descVersion = 0xea55,
>     tagChecksum = 0xcd,
>     reserved = 0x66,
>     tagSerialNum = 0x511f,
>     descCRC = 0x5a9c,
>     descCRCLength = 0x5142,
>     tagLocation = 0x373ce06a
>   },
>   fileVersionNum = 0x3139,
>   fileCharacteristics = 0xf6,
>   lengthFileIdent = 0x7e,
>   icb = {
>     extLength = 0x6059792,
>     extLocation = {
>       logicalBlockNum = 0x73886466,
>       partitionReferenceNum = 0x7cc6
>     },
>     impUse = {0x3c, 0xcc, 0x4a, 0xed, 0xdc, 0xfb}
>   },
>   lengthOfImpUse = 0x1a6a,
>   impUse = 0xffff888019ca716a
> }
> ```
> These data are wrong and all data are part of  udf image mounted.
> Then next it would invoke function udf_write_fi to write fileident into `fi`.
> ```
> if (fileident) {
>       if (adinicb || (offset + lfi < 0)) {
>           memcpy(udf_get_fi_ident(sfi), fileident, lfi);   [3]
> } else if (offset >= 0) {
>           memcpy(fibh->ebh->b_data + offset, fileident, lfi);
> } else {
>           memcpy(udf_get_fi_ident(sfi), fileident, -offset);
>           memcpy(fibh->ebh->b_data, fileident - offset,
>                   lfi + offset);
>        }
> }
> ```
> The fileident was controlled by user. `sfi` is `fi`, the memcpy
> function is called to copy the data, so an out-of-bounds write occurs

Thanks for report and the analysis. This is indeed a serious bug. I'll send
a fix shortly.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR