SIGABRT in src/jp2image.cpp Safe::add

[legend-issue]

RAX: 0x0 
RBX: 0xec0d18 ("Overflow in addition")
RCX: 0x7ffff693b428 (<__GI_raise+56>:	cmp    rax,0xfffffffffffff000)
RDX: 0x6 
RSI: 0x9367 
RDI: 0x9367 
RBP: 0xe907c0 --> 0x7ffff6ccb540 --> 0xfbad2887 
RSP: 0x7fffffffcf78 --> 0x7ffff693d02a (<__GI_abort+362>:	mov    rdx,QWORD PTR fs:0x10)
RIP: 0x7ffff693b428 (<__GI_raise+56>:	cmp    rax,0xfffffffffffff000)
R8 : 0x7ffff6ccc770 --> 0x0 
R9 : 0x7ffff7fd3740 (0x00007ffff7fd3740)
R10: 0x8 
R11: 0x202 
R12: 0xec0aa0 --> 0x0 
R13: 0xba6958 --> 0x4b40e0 (<Exiv2::FileIo::~FileIo()>:	lea    rsp,[rsp-0x98])
R14: 0x7fffffffd260 --> 0x14 
R15: 0x14
EFLAGS: 0x202 (carry parity adjust zero sign trap INTERRUPT direction overflow)
[-------------------------------------code-------------------------------------]
   0x7ffff693b41e <__GI_raise+46>:	mov    eax,0xea
   0x7ffff693b423 <__GI_raise+51>:	movsxd rdi,ecx
   0x7ffff693b426 <__GI_raise+54>:	syscall 
=> 0x7ffff693b428 <__GI_raise+56>:	cmp    rax,0xfffffffffffff000
   0x7ffff693b42e <__GI_raise+62>:	ja     0x7ffff693b450 <__GI_raise+96>
   0x7ffff693b430 <__GI_raise+64>:	repz ret 
   0x7ffff693b432 <__GI_raise+66>:	nop    WORD PTR [rax+rax*1+0x0]
   0x7ffff693b438 <__GI_raise+72>:	test   ecx,ecx
[------------------------------------stack-------------------------------------]
0000| 0x7fffffffcf78 --> 0x7ffff693d02a (<__GI_abort+362>:	mov    rdx,QWORD PTR fs:0x10)
0008| 0x7fffffffcf80 --> 0x20 (' ')
0016| 0x7fffffffcf88 --> 0x0 
0024| 0x7fffffffcf90 --> 0x0 
0032| 0x7fffffffcf98 --> 0x0 
0040| 0x7fffffffcfa0 --> 0x0 
0048| 0x7fffffffcfa8 --> 0x0 
0056| 0x7fffffffcfb0 --> 0x0 
[------------------------------------------------------------------------------]
Legend: code, data, rodata, value
Stopped reason: SIGABRT
0x00007ffff693b428 in __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:54
54	../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
[ Legend: Modified register | Code | Heap | Stack | String ]
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────[ registers ]────
$rax   : 0x0000000000000000
$rbx   : 0x0000000000ec0d18  →  "Overflow in addition"
$rcx   : 0x00007ffff693b428  →  0x2077fffff0003d48 ("H="?)
$rdx   : 0x0000000000000006
$rsp   : 0x00007fffffffcf78  →  0x00007ffff693d02a  →  <abort+362> mov rdx, QWORD PTR fs:0x10
$rbp   : 0x0000000000e907c0  →  0x00007ffff6ccb540  →  0x00000000fbad2887
$rsi   : 0x0000000000009367
$rdi   : 0x0000000000009367
$rip   : 0x00007ffff693b428  →  0x2077fffff0003d48 ("H="?)
$r8    : 0x00007ffff6ccc770  →  0x0000000000000000
$r9    : 0x00007ffff7fd3740  →  0x00007ffff7fd3740  →  [loop detected]
$r10   : 0x0000000000000008
$r11   : 0x0000000000000202
$r12   : 0x0000000000ec0aa0  →  0x0000000000000000
$r13   : 0x0000000000ba6958  →  0x00000000004b40e0  →  <Exiv2::FileIo::~FileIo()+0> lea rsp, [rsp-0x98]
$r14   : 0x00007fffffffd260  →  0x0000000000000014
$r15   : 0x0000000000000014
$eflags: [carry parity adjust zero sign trap INTERRUPT direction overflow resume virtualx86 identification]
$gs: 0x0000  $fs: 0x0000  $ss: 0x002b  $ds: 0x0000  $es: 0x0000  $cs: 0x0033  
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────[ stack ]────
0x00007fffffffcf78│+0x00: 0x00007ffff693d02a  →  <abort+362> mov rdx, QWORD PTR fs:0x10	 ← $rsp
0x00007fffffffcf80│+0x08: 0x0000000000000020
0x00007fffffffcf88│+0x10: 0x0000000000000000
0x00007fffffffcf90│+0x18: 0x0000000000000000
0x00007fffffffcf98│+0x20: 0x0000000000000000
0x00007fffffffcfa0│+0x28: 0x0000000000000000
0x00007fffffffcfa8│+0x30: 0x0000000000000000
0x00007fffffffcfb0│+0x38: 0x0000000000000000
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────[ code:i386:x86-64 ]────
   0x7ffff693b41e <raise+46>       mov    eax, 0xea
   0x7ffff693b423 <raise+51>       movsxd rdi, ecx
   0x7ffff693b426 <raise+54>       syscall 
 → 0x7ffff693b428 <raise+56>       cmp    rax, 0xfffffffffffff000
   0x7ffff693b42e <raise+62>       ja     0x7ffff693b450 <__GI_raise+96>
   0x7ffff693b430 <raise+64>       repz   ret
   0x7ffff693b432 <raise+66>       nop    WORD PTR [rax+rax*1+0x0]
   0x7ffff693b438 <raise+72>       test   ecx, ecx
   0x7ffff693b43a <raise+74>       jg     0x7ffff693b41b <__GI_raise+43>
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────[ threads ]────
[#0] Id 1, Name: "exiv2", stopped, reason: SIGABRT
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────[ trace ]────
[#0] 0x7ffff693b428 → Name: __GI_raise(sig=0x6)
[#1] 0x7ffff693d02a → Name: __GI_abort()
[#2] 0x7ffff727e84d → Name: __gnu_cxx::__verbose_terminate_handler()()
[#3] 0x7ffff727c6b6 → call 0x7ffff7275fc0 <abort@plt>
[#4] 0x7ffff727c701 → Name: std::terminate()()
[#5] 0x7ffff727c919 → Name: __cxa_throw()
[#6] 0x607c6a → Name: Safe::add<unsigned int>(summand_2=0x8, summand_1=<optimized out>)
[#7] 0x607c6a → Name: Exiv2::Jp2Image::readMetadata(this=<optimized out>)
[#8] 0x47ab61 → Name: Action::Extract::writeThumbnail(this=0xec1cf0)
[#9] 0x496fa0 → Name: Action::Extract::run(this=0xec1cf0, path="/home/aflgo/exiv2/out-2/crashes/id:000008,sig:06,src:000335,op:int32,pos:62,val:-1")

command: exiv2 -et [poc]
https://github.com/legend-issue/pocs/blob/master/exiv2/id:000008%2Csig:06%2Csrc:000335%2Cop:int32%2Cpos:62%2Cval:-1

[fgeek]

I can't reproduce this issue. I only receive following:

std::overflow_error exception in print action for file id:000008,sig:06,src:000335,op:int32,pos:62,val:-1:
Overflow in addition

In what git commit are you executing?

[legend-issue]

I think you need to set virtual memory to 1G. "ulimit -v 1048576" @fgeek

[fgeek]

Can't reproduce and it is completely normal that program like exiv2 takes lots amount of memory when you are executing it on fuzzed samples. These are denial of service issues in some context dependent cases, but I don't think there is any issue in this case. I tested in the latest commit bb20191.

[fgeek]

CVE-2018-10998 has been assigned for this issue (not requested by me), which I think should be REJECTED. It would be nice @legend-issue if you would first wait for upstream response before requesting CVEs and you should always focus your fuzzing efforts to the latest commit when fuzzing software like exiv2, which is quite heavily fuzzed already.

[piponazo]

Hi guys, thanks for the report.

I just want to confirm that I could reproduce the issue and I get this:

09:07 $ bin/exiv2 -et ~/Downloads/exiv2/id_000008\,sig_06\,src_000335\,op_int32\,pos_62\,val_-1 
terminate called after throwing an instance of 'std::overflow_error'
  what():  Overflow in addition
Aborted (core dumped)

[piponazo]

Hum ... wait a second. Our code is throwing an exception intentionally in this piece of code:

    template <typename T>
    T add(T summand_1, T summand_2)
    {
        T res = 0;
        if (Internal::builtin_add_overflow(summand_1, summand_2, res)) {
            throw std::overflow_error("Overflow in addition");
        }
        return res;
    }

And later the exiv2 app is not catching that exception.

Honestly I do not know much about Linux security issues. I never thought if letting an exception reach the main() function would cause any risks from the security point of view. Should we try to catch that exception and exit gracefully? I would appreciate if you point me to some resources where I can learn about this topic.

[D4N]

Letting an exception through is bad style, as an uncaught exception results in a call to std::abort which does not call any destructors (in theory bad stuff™ could happen, but imho nothing too serious, at worst an app calling exiv2 repeatedly could run out of file descriptors, but that's about it).

I guess we should just catch the exception and return with 0.

@legend-issue Please read the error message next time. This issue caused far too much noise than necessary, as the exception prevented actual bad stuff™ from happening.

[piponazo]

That is what I have always believe but I would also to know their opinion about this (I'm sure they know much more than me in that topic 🙈 )

When debugging the issue I thought about catching the exception at this point

    int Extract::writeThumbnail() const
    {
        if (!Exiv2::fileExists(path_, true)) {
            std::cerr << path_
                      << ": " << _("Failed to open the file\n");
            return -1;
        }
        Exiv2::Image::AutoPtr image = Exiv2::ImageFactory::open(path_);
        assert(image.get() != 0);
        try {
            image->readMetadata();  // The exception is thrown within this function
        } catch (const std::exception &e) {
            std::cerr << "Exception caught while reading metadata: " << e.what() << std::endl;
            return -1;
        }
...
    }

[D4N]

The exception catching has been added by f4e8ed2. Exiv2 will now report that an exception was not caught by any of the actions and print its what() message.