#850 closed defect (fixed)

Off by one bug in src/lib/mr_str.c

Reported by: KONG Owned by: Bruno Cornec
Priority: high Milestone: 3.3.0
Component: mondo Version: 3.2.2
Severity: major Keywords:
Cc:

Description

I found a bug in src/lib/mr_str.c. The following line:

q = outstr + strlen(outstr) -1;

will cause a SIGABRT on latest rhel 7. See parts of the log:

*** Error in `/usr/sbin/mondoarchive': corrupted size vs. prev_size: 0x000000000065b5d0 ***
======= Backtrace: =========
/lib64/libc.so.6(+0x7f7c4)[0x7ffff765b7c4]
/lib64/libc.so.6(+0x82fd4)[0x7ffff765efd4]
/lib64/libc.so.6(+0x840c2)[0x7ffff76600c2]
/lib64/libc.so.6(realloc+0x1d2)[0x7ffff7662162]
/lib64/libc.so.6(getdelim+0x10b)[0x7ffff764b91b]
/usr/sbin/mondoarchive[0x4380bf]
/usr/sbin/mondoarchive[0x41e765]
/usr/sbin/mondoarchive[0x4291c8]
/usr/sbin/mondoarchive[0x402e90]
/lib64/libc.so.6(__libc_start_main+0xf5)[0x7ffff75fe505]
/usr/sbin/mondoarchive[0x4039b3]
==
....

Running it through valgrind, displays:

I think you have a EFI/UEFI partition.
==2587== Invalid read of size 1
==2587==    at 0x437DAA: mr_strip_spaces (mr_str.c:185)
==2587==    by 0x41E6FC: run_program_and_log_output (libmondo-fork.c:317)
==2587==    by 0x42B079: some_basic_system_sanity_checks (libmondo-tools.c:1016)
==2587==    by 0x42B4A3: pre_param_configuration (libmondo-tools.c:684)
==2587==    by 0x402E46: main (mondoarchive.c:328)
==2587==  Address 0x5ed05bf is 1 bytes before a block of size 1 alloc'd
==2587==    at 0x4C29EA3: malloc (vg_replace_malloc.c:309)
==2587==    by 0x537FF2F: __vasprintf_chk (vasprintf_chk.c:80)
==2587==    by 0x437FFA: UnknownInlinedFun (stdio2.h:210)
==2587==    by 0x437FFA: mr_asprintf_int (mr_mem.c:62)
==2587==    by 0x437D98: mr_strip_spaces (mr_str.c:181)
==2587==    by 0x41E6FC: run_program_and_log_output (libmondo-fork.c:317)
==2587==    by 0x42B079: some_basic_system_sanity_checks (libmondo-tools.c:1016)
==2587==    by 0x42B4A3: pre_param_configuration (libmondo-tools.c:684)
==2587==    by 0x402E46: main (mondoarchive.c:328)
==2587==

The following change fixes it:

q = outstr + strlen(outstr);

After that archive creation successfully completes.

Change History (3)

comment:1 Changed 11 months ago by Bruno Cornec

Priority: normalhigh
Severity: normalmajor
Status: newassigned

Thanks for your report.

Should be fixed by [3723].

Your proposed patch is IMO incorrect, except for empty strings, so it should crash elsewhere.

Please try applying mine to see if it is also fixing your case, without creating other issues.

comment:2 Changed 11 months ago by KONG

Just tested the fix, but I think you made an error. With your fix I get a message after Checking sanity of your linux distribution: "You must have at least 32MB of RAM to use Mondo".

But this server has 128GB of RAM

Your fix checks for instr != '\0' :

if (*instr != '\0') {
   mr_asprintf(outstr, "");
   return(outstr);
}

I think what you wanted to write is intstr == '\0':

if (*instr == '\0') {
   mr_asprintf(outstr, "");
   return(outstr);
}

With this archive creation works again as expected.

comment:3 Changed 11 months ago by Bruno Cornec

Resolution: fixed
Status: assignedclosed

Yes you're right ! Made the test wrong :-( Fixed then better with [3724]

Note: See TracTickets for help on using tickets.