ch0ic3/cutter
1
0
Fork 0
mirror of https://github.com/rizinorg/cutter.git synced 2024-12-18 19:06:10 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Fix errors reported by UB sanitizer (#2150)

Browse Source 
* Use qt parent for deleting completer

Manually deleting causes some UAF due to it being installed as event
filter.
Qt seems to destroy things in correct order. Sanitizer doesn't report
completer as being leaked.

* Fix sanitizer problems in HexWidget

* Initialize size properties to somewhat sane values to avoid
unrealized variable use when calculating them first time.
* Change AbstractData interface. Old one returned pointer to unknown
sized block of data which was difficult to use correctly. Adjust bound
checking to avoid out of bounds access when comparing with oldData and
scrolling.
This commit is contained in:
karliss 2020-04-16 20:32:24 +03:00 committed by GitHub
parent ccb53fedbf
commit 4d2ef58e6a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 62 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/widgets/ConsoleWidget.cpp
View File

@ -133,8 +133,6 @@ ConsoleWidget::ConsoleWidget(MainWindow *main, QAction *action) :
ConsoleWidget::~ConsoleWidget() ConsoleWidget::~ConsoleWidget()
{ {
delete completer;
#ifndef Q_OS_WIN #ifndef Q_OS_WIN
::close(stdinFile); ::close(stdinFile);
remove(stdinFifoPath.toStdString().c_str()); remove(stdinFifoPath.toStdString().c_str());

60
src/widgets/HexWidget.cpp
View File

@ -262,13 +262,11 @@ void HexWidget::setItemGroupSize(int size)
* It is assumed that the current read data buffer contains the address. * It is assumed that the current read data buffer contains the address.
*/ */
bool HexWidget::isItemDifferentAt(uint64_t address) { bool HexWidget::isItemDifferentAt(uint64_t address) {
if (address >= oldData->minIndex() && address < oldData->maxIndex()) { char oldItem[sizeof(uint64_t)] = {};
uint64_t itemOffset = address - startAddress; char newItem[sizeof(uint64_t)] = {};
QVariant curItem = readItem(itemOffset); if (data->copy(newItem, address, static_cast<size_t>(itemByteLen)) &&
data.swap(oldData); oldData->copy(oldItem, address, static_cast<size_t>(itemByteLen))) {
QVariant oldItem = readItem(itemOffset); return memcmp(oldItem, newItem, sizeof(oldItem)) != 0;
data.swap(oldData);
return (oldItem != curItem);
} }
return false; return false;
} }
@ -1373,15 +1371,20 @@ QVariant HexWidget::readItem(int offset, QColor *color)
quint16 word; quint16 word;
quint32 dword; quint32 dword;
quint64 qword; quint64 qword;
float *ptrFloat32; float float32;
double *ptrFloat64; double float64;
const void *dataPtr = data->dataPtr(startAddress + offset); quint8 bytes[sizeof(uint64_t)];
data->copy(bytes, startAddress + offset, static_cast<size_t>(itemByteLen));
const bool signedItem = itemFormat == ItemFormatSignedDec; const bool signedItem = itemFormat == ItemFormatSignedDec;
if (color) {
*color = defColor;
}
switch (itemByteLen) { switch (itemByteLen) {
case 1: case 1:
byte = *static_cast<const quint8 *>(dataPtr); byte = bytes[0];
if (color) if (color)
*color = itemColor(byte); *color = itemColor(byte);
if (!signedItem) if (!signedItem)
@ -1389,38 +1392,34 @@ QVariant HexWidget::readItem(int offset, QColor *color)
return QVariant(static_cast<qint64>(static_cast<qint8>(byte))); return QVariant(static_cast<qint64>(static_cast<qint8>(byte)));
case 2: case 2:
if (itemBigEndian) if (itemBigEndian)
word = fromBigEndian<quint16>(dataPtr); word = fromBigEndian<quint16>(bytes);
else else
word = fromLittleEndian<quint16>(dataPtr); word = fromLittleEndian<quint16>(bytes);
if (color)
*color = defColor;
if (!signedItem) if (!signedItem)
return QVariant(static_cast<quint64>(word)); return QVariant(static_cast<quint64>(word));
return QVariant(static_cast<qint64>(static_cast<qint16>(word))); return QVariant(static_cast<qint64>(static_cast<qint16>(word)));
case 4: case 4:
if (itemBigEndian) if (itemBigEndian)
dword = fromBigEndian<quint32>(dataPtr); dword = fromBigEndian<quint32>(bytes);
else else
dword = fromLittleEndian<quint32>(dataPtr); dword = fromLittleEndian<quint32>(bytes);
if (color)
*color = defColor;
if (itemFormat == ItemFormatFloat) { if (itemFormat == ItemFormatFloat) {
ptrFloat32 = static_cast<float *>(static_cast<void *>(&dword)); memcpy(&float32, &dword, sizeof(float32));
return QVariant(*ptrFloat32); return QVariant(float32);
} }
if (!signedItem) if (!signedItem)
return QVariant(static_cast<quint64>(dword)); return QVariant(static_cast<quint64>(dword));
return QVariant(static_cast<qint64>(static_cast<qint32>(dword))); return QVariant(static_cast<qint64>(static_cast<qint32>(dword)));
case 8: case 8:
if (itemBigEndian) if (itemBigEndian)
qword = fromBigEndian<quint64>(dataPtr); qword = fromBigEndian<quint64>(bytes);
else else
qword = fromLittleEndian<quint64>(dataPtr); qword = fromLittleEndian<quint64>(bytes);
if (color)
*color = defColor;
if (itemFormat == ItemFormatFloat) { if (itemFormat == ItemFormatFloat) {
ptrFloat64 = static_cast<double *>(static_cast<void *>(&qword)); memcpy(&float64, &qword, sizeof(float64));
return QVariant(*ptrFloat64); return QVariant(float64);
} }
if (!signedItem) if (!signedItem)
return QVariant(qword); return QVariant(qword);
@ -1462,7 +1461,8 @@ QString HexWidget::renderItem(int offset, QColor *color)
QChar HexWidget::renderAscii(int offset, QColor *color) QChar HexWidget::renderAscii(int offset, QColor *color)
{ {
uchar byte = *static_cast<const uint8_t *>(data->dataPtr(startAddress + offset)); uchar byte;
data->copy(&byte, startAddress + offset, sizeof(byte));
if (color) { if (color) {
*color = itemColor(byte); *color = itemColor(byte);
} }
@ -1539,7 +1539,7 @@ BasicCursor HexWidget::mousePosToAddr(const QPoint &point, bool middle) const
return asciiArea.contains(point) ? asciiPosToAddr(point, middle) : screenPosToAddr(point, middle); return asciiArea.contains(point) ? asciiPosToAddr(point, middle) : screenPosToAddr(point, middle);
} }
QRectF HexWidget::itemRectangle(uint offset) QRectF HexWidget::itemRectangle(int offset)
{ {
qreal x; qreal x;
qreal y; qreal y;
@ -1565,7 +1565,7 @@ QRectF HexWidget::itemRectangle(uint offset)
return QRectF(x, y, width, lineHeight); return QRectF(x, y, width, lineHeight);
} }
QRectF HexWidget::asciiRectangle(uint offset) QRectF HexWidget::asciiRectangle(int offset)
{ {
QPointF p; QPointF p;

49
src/widgets/HexWidget.h
View File

@ -75,7 +75,7 @@ class AbstractData
public: public:
virtual ~AbstractData() {} virtual ~AbstractData() {}
virtual void fetch(uint64_t addr, int len) = 0; virtual void fetch(uint64_t addr, int len) = 0;
virtual const void *dataPtr(uint64_t addr) = 0; virtual bool copy(void *out, uint64_t adr, size_t len) = 0;
virtual uint64_t maxIndex() = 0; virtual uint64_t maxIndex() = 0;
virtual uint64_t minIndex() = 0; virtual uint64_t minIndex() = 0;
}; };
@ -101,9 +101,12 @@ public:
void fetch(uint64_t, int) override { } void fetch(uint64_t, int) override { }
const void *dataPtr(uint64_t addr) override bool copy(void *out, uint64_t addr, size_t len) override {
{ if (addr < static_cast<uint64_t>(m_buffer.size()) && (static_cast<uint64_t>(m_buffer.size()) - addr) < len) {
return m_buffer.constData() + addr; memcpy(out, m_buffer.constData() + addr, len);
return true;
}
return false;
} }
uint64_t maxIndex() override uint64_t maxIndex() override
@ -120,6 +123,7 @@ class MemoryData : public AbstractData
public: public:
MemoryData() {} MemoryData() {}
~MemoryData() override {} ~MemoryData() override {}
static constexpr size_t BLOCK_SIZE = 4096;
void fetch(uint64_t address, int length) override void fetch(uint64_t address, int length) override
{ {
@ -141,12 +145,23 @@ public:
} }
} }
const void *dataPtr(uint64_t addr) override bool copy(void *out, uint64_t addr, size_t len) override {
{ if (addr < m_firstBlockAddr || addr > m_lastValidAddr ||
(m_lastValidAddr - addr + 1) < len /* do not merge with last check to handle overflows */) {
return false;
}
int totalOffset = addr - m_firstBlockAddr; int totalOffset = addr - m_firstBlockAddr;
int blockId = totalOffset / 4096; int blockId = totalOffset / BLOCK_SIZE;
int blockOffset = totalOffset % 4096; int blockOffset = totalOffset % BLOCK_SIZE;
return static_cast<const void *>(m_blocks.at(blockId).constData() + blockOffset); size_t first_part = BLOCK_SIZE - blockOffset;
if (first_part >= len) {
memcpy(out, m_blocks.at(blockId).constData() + blockOffset, len);
} else {
memcpy(out, m_blocks.at(blockId).constData() + blockOffset, first_part);
memcpy(static_cast<char*>(out) + first_part, m_blocks.at(blockId + 1).constData(), len - first_part);
}
return true;
} }
virtual uint64_t maxIndex() override virtual uint64_t maxIndex() override
@ -353,13 +368,13 @@ private:
* @param offset relative to first byte on screen * @param offset relative to first byte on screen
* @return * @return
*/ */
QRectF itemRectangle(uint offset); QRectF itemRectangle(int offset);
/** /**
* @brief Rectangle for single item in ascii area. * @brief Rectangle for single item in ascii area.
* @param offset relative to first byte on screen * @param offset relative to first byte on screen
* @return * @return
*/ */
QRectF asciiRectangle(uint offset); QRectF asciiRectangle(int offset);
QVector<QPolygonF> rangePolygons(RVA start, RVA last, bool ascii); QVector<QPolygonF> rangePolygons(RVA start, RVA last, bool ascii);
void updateWidth(); void updateWidth();
@ -450,12 +465,12 @@ private:
QRectF itemArea; QRectF itemArea;
QRectF asciiArea; QRectF asciiArea;
int itemByteLen; int itemByteLen = 1;
int itemGroupSize; ///< Items per group (default: 1), 2 in case of hexpair mode int itemGroupSize = 1; ///< Items per group (default: 1), 2 in case of hexpair mode
int rowSizeBytes; ///< Line size in bytes int rowSizeBytes = 16; ///< Line size in bytes
int itemColumns; ///< Number of columns, single column consists of itemGroupSize items int itemColumns = 16; ///< Number of columns, single column consists of itemGroupSize items
int itemCharLen; int itemCharLen = 2;
int itemPrefixLen; int itemPrefixLen = 0;
ColumnMode columnMode; ColumnMode columnMode;
ItemFormat itemFormat; ItemFormat itemFormat;