Opened 7 years ago
Last modified 7 years ago
#5754 new Bug
Potential crash-causing overflow in blockCountInPiece
| Reported by: | x190 | Owned by: | jordan |
|---|---|---|---|
| Priority: | Normal | Milestone: | None Set |
| Component: | libtransmission | Version: | 2.84 |
| Severity: | Normal | Keywords: | |
| Cc: |
Description (last modified by jordan)
Currently in torrent.h, "blockCountInPiece" is an uint16_t. Torrents can be crafted with a piece size that will overflow this field and cause a divide by 0 crash in torrent.h's tr_torBlockPiece().
See #4005#comment:16 and #4005#comment:23 for proof of concept and test torrents.
Change History (3)
comment:1 Changed 7 years ago by jordan
- Description modified (diff)
comment:2 follow-up: ↓ 3 Changed 7 years ago by jordan
comment:3 in reply to: ↑ 2 Changed 7 years ago by cfpp2p
Replying to jordan:
Yeah, looks like it would be reasonable to widen blockCountInPiece and blockCountInLastPiece to uint32_t.
...actually, it doesn't appear that "blockCountInLastPiece" has any useful role in libtransmission anymore. If we remove it at the same time, that will offset widening blockCountInPiece's effect on sizeof(tr_torrent) ;)
This topic seemed to generate a lot of discussion in bug #4005... anyone disagree with this idea?
I think the discussion at ticket:5736#comment:4 might clarify my opinion :)
Yeah, looks like it would be reasonable to widen blockCountInPiece and blockCountInLastPiece to uint32_t.
...actually, it doesn't appear that "blockCountInLastPiece" has any useful role in libtransmission anymore. If we remove it at the same time, that will offset widening blockCountInPiece's effect on sizeof(tr_torrent) ;)
This topic seemed to generate a lot of discussion in bug #4005... anyone disagree with this idea?