CPIO_CRCASCII checksum handling in Lua

[Storm, Christian]

Hi,

while wading through the code for the short read/early EOF fix, I found that the C code for image:copy2file() and image:read() to verify the CPIO checksum in corelib/lua_interface.c is dead/broken:

static int l_istream_read(lua_State* L)
[...]
struct img_type img = {};
uint32_t image_checksum = img.checksum;
[...]
if ((image_checksum != 0) && (image_checksum != img.checksum)) {
lua_pushinteger(L, -1);
lua_pushfstring(L, "Checksums WRONG! Computed 0x%d, should be 0x%d\n",
img.checksum, image_checksum);
return 2;
}
[...]

Here, image_checksum is initialized to 0 and it cannot become != 0 so that the if (...) branch cannot be taken, i.e., it is dead code. The same holds true for l_copy2file().

For COPY_FILE and SKIP_FILE, the CPIO checksum for CPIO_CRCASCII files is checked in the C realm in core/stream_interface.c's extract_files() by calling swupdate_verify_chksum(), so no need to check it in the Lua realm (again). In the INSTALL_FROM_STREAM case, the CPIO checksum is currently not checked, neither for Lua nor for C handlers.

So, should we better remove that dead code in corelib/lua_interface.c or introduce proper checking (probably also for C handlers for symmetry)? Given that we have HASH_VERIFY, I tend to just removing that dead code.

What's your take on this?


Thanks for reading and please correct me if I got this wrong :)


Kind regards,
Christian

--
Dr. Christian Storm
Siemens AG, FT RPD CED
Friedrich-Ludwig-Bauer-Str. 3, 85748 Garching, Germany

[Stefano Babic]

Hi Christian,

On 8/4/26 10:58, 'Storm, Christian' via swupdate wrote:
> Hi,
>
> while wading through the code for the short read/early EOF fix, I found that the C code for image:copy2file() and image:read() to verify the CPIO checksum in corelib/lua_interface.c is dead/broken:
>
> static int l_istream_read(lua_State* L)
> [...]
> struct img_type img = {};
> uint32_t image_checksum = img.checksum;
> [...]
> if ((image_checksum != 0) && (image_checksum != img.checksum)) {
> lua_pushinteger(L, -1);
> lua_pushfstring(L, "Checksums WRONG! Computed 0x%d, should be 0x%d\n",
> img.checksum, image_checksum);
> return 2;
> }
> [...]
>
> Here, image_checksum is initialized to 0 and it cannot become != 0 so that the if (...) branch cannot be taken, i.e., it is dead code. The same holds true for l_copy2file().
>
> For COPY_FILE and SKIP_FILE, the CPIO checksum for CPIO_CRCASCII files is checked in the C realm in core/stream_interface.c's extract_files() by calling swupdate_verify_chksum(), so no need to check it in the Lua realm (again).

Right.

> In the INSTALL_FROM_STREAM case, the CPIO checksum is currently not checked, neither for Lua nor for C handlers.
>
> So, should we better remove that dead code in corelib/lua_interface.c or introduce proper checking (probably also for C handlers for symmetry)? Given that we have HASH_VERIFY, I tend to just removing that dead code.

The CRC check is in any case a weak mechanism, but it is provided out of
the box by the CPIO format and it was easy to implement it inside
SWUpdate. It does not make any integrity check, and it is also easy to
skip it if a malicious wants it.

Later I introduced in SWUpdate a dynamic check based on the CPIO header,
and check is done only in case of CPIO_CRCASCII and skipped in new ascii
format is chosen in SWU. Note that the both swugenerator and
meta-swupdate create a CPIO in new ascii format, so to enable the CRC
someone should create the SWU by packing directly via cpio command.

That is I am sure in quite all current projects, CRC check is skipped.
And all current projects set on signed images, that is hash verification
is on.

IMHO it is better to drop this dead code.

Best regards,
Stefano

>
> What's your take on this?
>
>
> Thanks for reading and please correct me if I got this wrong :)
>
>
> Kind regards,
> Christian
>

--

_______________________________________________________________________
Nabla Software Engineering GmbH
Hirschstr. 111A | 86156 Augsburg | Tel: +49 821 45592596
Geschäftsführer : Stefano Babic | HRB 40522 Augsburg
E-Mail: sba...@nabladev.com