Skip to content

catch stoul exceptions in CanonMakerNote print0x000a/print0x000c (backport #9320) - #9372

Open
mergify[bot] wants to merge 3 commits into
0.28.xfrom
mergify/bp/0.28.x/pr-9320
Open

catch stoul exceptions in CanonMakerNote print0x000a/print0x000c (backport #9320)#9372
mergify[bot] wants to merge 3 commits into
0.28.xfrom
mergify/bp/0.28.x/pr-9320

Conversation

@mergify

@mergify mergify Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

print0x000a and print0x000c call std::stoul(value.toString()), but the stored type of Canon tags 0x000a and 0x000c is not enforced during parsing, so a crafted makernote can hand them a non-numeric ASCII value. stoul then throws std::invalid_argument, which Exifdatum::write does not catch (it only guards std::out_of_range), so it escapes during -pa printing. Catch both and fall back to the raw value, like the existing path in print0x000c.


This is an automatic backport of pull request #9320 done by Mergify.

@mergify mergify Bot added the conflicts label Jul 1, 2026
@mergify

mergify Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-pick of f62ab78 has failed:

On branch mergify/bp/0.28.x/pr-9320
Your branch is up to date with 'origin/0.28.x'.

You are currently cherry-picking commit f62ab784.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   src/canonmn_int.cpp

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@kevinbackhouse
kevinbackhouse force-pushed the mergify/bp/0.28.x/pr-9320 branch from 05131c3 to 5502e98 Compare July 4, 2026 21:07
Comment thread src/canonmn_int.cpp
is >> l;
return os << std::setw(4) << std::setfill('0') << std::hex << ((l & 0xffff0000) >> 16) << std::setw(5)
<< std::setfill('0') << std::dec << (l & 0x0000ffff);
} catch (const std::logic_error&) {

@kmilos kmilos Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::istringstream seems to throw only std::ios_base::failure, so this should perhaps be the base std::system_error?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't even find any documentation that says what exceptions it might throw! But I did find this, which looks like a simpler solution: https://en.cppreference.com/cpp/io/basic_ios/exceptions. So I think we can just do this and it won't throw any exceptions:

is.exceptions(0);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't even find any documentation that says what exceptions it might throw!

See https://en.cppreference.com/cpp/io/ios_base/failure

I'm fine with either masking or catching std::system_error.

@kmilos kmilos Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw https://cplusplus.com/reference/ios/ios/exceptions/ says the mask is 0 by default, so this PR might even be unnecessary on 0.28.x?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tested it (using #9397) and it looks like std::logic_error is correct. When I tried replacing it with std::system_error the exception didn't get caught.

gdb) run
Starting program: /home/kev/work/exiv2b/build/bin/exiv2 -pa /home/kev/work/exiv2b/test/data/pr_9320.jpg
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/x86_64-linux-gnu/libthread_db.so.1".

Breakpoint 1, Exiv2::Internal::CanonMakerNote::print0x000c (os=..., value=..., exifData=0x555555628420) at /home/kev/work/exiv2b/src/canonmn_int.cpp:2738
2738          return os << value;
(gdb) p e
$1 = (const std::logic_error &) @0x5555556376a0: <incomplete type>
(gdb) 

Comment thread src/canonmn_int.cpp
is >> l;
return os << std::setw(4) << std::setfill('0') << std::hex << ((l & 0xffff0000) >> 16) << std::setw(5)
<< std::setfill('0') << std::dec << (l & 0x0000ffff);
} catch (const std::logic_error&) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

@kevinbackhouse

Copy link
Copy Markdown
Collaborator

I'm adding a regression test so that we can verify which exception we need to catch here.
#9397

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants