[Bug] remove use of variable length buffer #494
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
assigned
rejectedpromise
force-pushed
the
bufferoverflowfix
branch
from
e999389 to
d4e768b
Compare
Collaborator
|
utACK d4e768b AFAIK this was always more of a compile-time warning (not error) on some systems. Still needs solid testing and verification that nothing breaks during runtime. |
Mrs-X
suggested changes
Jan 20, 2018
Mrs-X
left a comment
Mrs-X
left a comment
There was a problem hiding this comment.
utACK that is removes the compile-time warning, but NACK for the paymentserver-test itself, it fails now:
Before your change:
==========================================
Pivx Core 3.0.99: src/test-suite.log
==========================================
# TOTAL: 3
# PASS: 3
# SKIP: 0
# XFAIL: 0
# FAIL: 0
# XPASS: 0
# ERROR: 0
.. contents:: :depth: 2
After:
==========================================
Pivx Core 3.0.99: src/test-suite.log
==========================================
# TOTAL: 3
# PASS: 2
# SKIP: 0
# XFAIL: 0
# FAIL: 1
# XPASS: 0
# ERROR: 0
.. contents:: :depth: 2
The error itself:
********* Start testing of PaymentServerTests *********
Config: Using QtTest library 5.5.1, Qt 5.5.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 5.4.0 20160609)
PASS : PaymentServerTests::initTestCase()
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::initNetManager : No active proxy server found.
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::processPaymentRequest : Secure payment request from "testmerchant.org"
QWARN : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant : Payment request: certificate expired or not yet active: QSslCertificate("3", "03", "LxHILx+N3qwVoAcCmQ5cyw==", (), ("Expired Test Merchant"), QMap(), QDateTime(2013-02-23 21:26:43.000 UTC Qt::TimeSpec(UTC)), QDateTime(2013-02-24 21:26:43.000 UTC Qt::TimeSpec(UTC)))
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::processPaymentRequest : Insecure payment request to "DJkvsVmdCTtpeTG1ChQuQ3Q1A8mMbKdAaL"
QWARN : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant : Payment request: certificate expired or not yet active: QSslCertificate("3", "03", "LxHILx+N3qwVoAcCmQ5cyw==", (), ("Expired Test Merchant"), QMap(), QDateTime(2013-02-23 21:26:43.000 UTC Qt::TimeSpec(UTC)), QDateTime(2013-02-24 21:26:43.000 UTC Qt::TimeSpec(UTC)))
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::processPaymentRequest : Secure payment request from "testmerchant8.org"
QWARN : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant : Payment request: certificate expired or not yet active: QSslCertificate("3", "06", "MiZaQ+g9lSHZGuHWkXZG+g==", (), ("Payment Request Intermediate 5"), QMap(), QDateTime(2013-02-23 22:59:51.000 UTC Qt::TimeSpec(UTC)), QDateTime(2013-02-24 22:59:51.000 UTC Qt::TimeSpec(UTC)))
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::processPaymentRequest : Insecure payment request to "DJkvsVmdCTtpeTG1ChQuQ3Q1A8mMbKdAaL"
QWARN : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant : Payment request: certificate expired or not yet active: QSslCertificate("3", "06", "MiZaQ+g9lSHZGuHWkXZG+g==", (), ("Payment Request Intermediate 5"), QMap(), QDateTime(2013-02-23 22:59:51.000 UTC Qt::TimeSpec(UTC)), QDateTime(2013-02-24 22:59:51.000 UTC Qt::TimeSpec(UTC)))
QWARN : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant : SSL error: certificate signature failure
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::processPaymentRequest : Insecure payment request to "DJkvsVmdCTtpeTG1ChQuQ3Q1A8mMbKdAaL"
QWARN : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant : SSL error: certificate signature failure
QWARN : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant : SSL error: unable to get local issuer certificate
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::processPaymentRequest : Insecure payment request to "DJkvsVmdCTtpeTG1ChQuQ3Q1A8mMbKdAaL"
QWARN : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant : SSL error: unable to get local issuer certificate
QFATAL : PaymentServerTests::paymentServerTests() Received signal 11
FAIL! : PaymentServerTests::paymentServerTests() Received a fatal error.
Loc: [Unknown file(0)]
Totals: 1 passed, 1 failed, 0 skipped, 0 blacklisted
********* Finished testing of PaymentServerTests *********
d4e768b to
29757a9
Compare
Author
|
@Mrs-X please run again and let me know if test now passes. |
rejectedpromise
force-pushed
the
bufferoverflowfix
branch
from
29757a9 to
d0e6e6b
Compare
|
Null-terminating is always a good idea :-) ACK and merging... |
Mrs-X
approved these changes
Jan 20, 2018
Mrs-X
added a commit
that referenced
this pull request
Jan 20, 2018
d0e6e6b fix paymentservertests.cpp to use automatic buffer overflow protection (rejectedpromise) Tree-SHA512: b6cc56afb452544a0b20ebe21f1464ab906c786221fcc779f77f241ef26f0d61a7d86c859060daf8e958af6c6ed7a9887542f4719dfeaaf03733eb39e45fb14d
Author
|
I think I was being a big dummy last night, probably tired too ;), and running test_pivx not test_pivxqt 🤦♂️ |
random-zebra
added a commit
to random-zebra/PIVX
that referenced
this pull request
Feb 13, 2021
Partially reverting PIVX-Project#494
Fuzzbawls
added a commit
that referenced
this pull request
Feb 14, 2021
84b4185 [Tests] raise minimum fee in feature_blockindexstats (random-zebra) 39d8a20 [Cleanup] Remove OldSetKeyFromPassphrase/OldEncrypt/OldDecrypt (random-zebra) 613e1da [BUG] Fix sizeof in paymentservertests (random-zebra) 0f3e961 [Cleanup] Remove useless call (random-zebra) b1ca5e0 Remove unused fsbridge::freopen (practicalswift) b3a1d84 Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) (practicalswift) ecfbcfd [Cleanup] Remove unused function AddInvalidSpendsToMap (random-zebra) 15b018c [Refactor] Use InsecureRand in the unit tests (random-zebra) 81fd84c [Refactoring] Replace risky call to rand() with GetRandInt() (random-zebra) 8ba1978 [Qt] Terminate string *pszExePath after readlink and without using memset (practicalswift) b6cd719 Remove unreachable code (g_rpcSignals.PostCommand) (practicalswift) e4f9ab3 [BUG] Memory leak after new CNode / ConnectNode (random-zebra) 536122b Avoid rollingMinimumFeeRate never being able to decay below half (Alex Morcos) 13cad19 fix a bug if the min fee is 0 for FeeFilterRounder (Alex Morcos) aa832d8 [Refactoring] Dereference before/after null check (random-zebra) 4630b7d [Trivial] Pass big parameters by reference, not value (random-zebra) aa7bc7f [Refactoring] Remove unneeded CScript::IsNormalPaymentScript (random-zebra) 6cf3c8f [Trivial] Unitialized scalar fields and variables (random-zebra) Pull request description: This is a collection of small fixes for the following issues/defects (discovered with the coverity tool): - Big parameters passed by value - calls to rand() function - Dereference before/after null-pointer check - Pointer to local variable out of scope - Resource leak with call to ConnectNode - Non-floating point result - unintended integer division (bitcoin#9288) - String not-null-terminated (bitcoin#11193) - Structurally dead code (bitcoin#9575) - Unchecked boolean return value of functions (GetOp, GetTransaction, TxOutToPublicCoin) - Unitialized pointers and scalar fields - Illegal memory access (bitcoin#13159) - Useless call to pubkey.IsCompressed() - Wrong `sizeof` argument (bitcoin#11024 + revert #494) ACKs for top commit: furszy: Looking good, ACK 84b4185 Fuzzbawls: ACK 84b4185 Tree-SHA512: 4c920a1858ccde65795c090e4becc47c50c0a78db7ab11935b4d3e2bea3f7f1f8ca3b48ce633d8112673092c35ae7cd05c444ed2b16e283a305c765874e55c1c
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove use of variable length buffer from
paymentservertests.cpp.This removes the warning of:
stack protector not protecting local variables: variable length bufferraised by the
-Wstack-protectorcompilation flag.As noted in issue: #387