Opened 4 years ago

Closed 4 years ago

#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 by Bruno Cornec, 4 years ago

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 by KONG, 4 years ago

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 by Bruno Cornec, 4 years ago

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.