Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -504,6 +504,14 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
return set_error(serror, SCRIPT_ERR_MINIMALDATA);
}
stack.push_back(vchPushValue);
+ if ((flags & SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS) && opcode == OP_FALSE) {
+ auto pc_tmp = pc;
+ opcodetype next_opcode;
+ valtype dummy_data;
+ if (script.GetOp(pc_tmp, next_opcode, dummy_data) && next_opcode == OP_IF) {
+ return set_error(serror, SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS);
+ }
+ }
} else if (fExec || (OP_IF <= opcode && opcode <= OP_ENDIF))
switch (opcode)
{
@luke-jr
Copy link
Author

luke-jr commented Feb 1, 2023

WARNING: This has NOT been tested. Use at your own risk. If you test it, please share your results.

Edit: Thanks to @LarryRuane for testing, this seems to work!

@LarryRuane
Copy link

I applied your patch to latest bitcoin core (no conflicts), and it did detect one transaction. But I don't know if it's an instance of the intended kind of transaction. I knew your code detected it because I set a breakpoint. Here's the stack backtrace (in case this is at all helpful):

#0  EvalScript (stack=std::vector of length 2, capacity 2 = {...}, script=..., flags=2097119, checker=..., sigversion=SigVersion::TAPSCRIPT, execdata=..., serror=0x7fff0ebd6f9c) at script/interpreter.cpp:488
#1  0x000055555611973e in ExecuteWitnessScript (stack_span=..., exec_script=..., flags=2097119, sigversion=SigVersion::TAPSCRIPT, checker=..., execdata=..., serror=0x7fff0ebd6f9c) at script/interpreter.cpp:1830
#2  0x000055555611a700 in VerifyWitnessProgram (witness=..., witversion=1, program=std::vector of length 32, capacity 32 = {...}, flags=2097119, checker=..., serror=0x7fff0ebd6f9c, is_p2sh=false) at script/interpreter.cpp:1944
#3  0x000055555611aae8 in VerifyScript (scriptSig=..., scriptPubKey=..., witness=0x7fff05467b30, flags=2097119, checker=..., serror=0x7fff0ebd6f9c) at script/interpreter.cpp:2001
#4  0x0000555555a90cd7 in CScriptCheck::operator() (this=0x7fff0ebd6f60) at ./src/validation.cpp:1645
#5  0x0000555555a914a3 in CheckInputScripts (tx=..., state=..., inputs=..., flags=2097119, cacheSigStore=true, cacheFullScriptStore=false, txdata=..., pvChecks=0x0) at ./src/validation.cpp:1740
#6  0x0000555555a8ab1f in (anonymous namespace)::MemPoolAccept::PolicyScriptChecks (this=0x7fff0ebd7760, args=..., ws=...) at ./src/validation.cpp:992
#7  0x0000555555a8c7d6 in (anonymous namespace)::MemPoolAccept::AcceptSingleTransaction (this=0x7fff0ebd7760, ptx=std::shared_ptr<const CTransaction> (use count 2, weak count 0) = {...}, args=...) at ./src/validation.cpp:1175
#8  0x0000555555a8ed9d in AcceptToMemoryPool (active_chainstate=..., tx=std::shared_ptr<const CTransaction> (use count 2, weak count 0) = {...}, accept_time=1675367849, bypass_limits=false, test_accept=false) at ./src/validation.cpp:1417
#9  0x0000555555aa24f0 in ChainstateManager::ProcessTransaction (this=0x5555568aad40, tx=std::shared_ptr<const CTransaction> (use count 2, weak count 0) = {...}, test_accept=false) at ./src/validation.cpp:3933
#10 0x0000555555694c6c in (anonymous namespace)::PeerManagerImpl::ProcessMessage (this=0x555563f49ea0, pfrom=..., msg_type=\"tx\", vRecv=..., time_received=..., interruptMsgProc=std::atomic<bool> = { false }) at ./src/net_processing.cpp:4045
#11 0x000055555569d9e1 in (anonymous namespace)::PeerManagerImpl::ProcessMessages (this=0x555563f49ea0, pfrom=0x7ffef0047700, interruptMsgProc=std::atomic<bool> = { false }) at ./src/net_processing.cpp:4926
#12 0x0000555555622b69 in CConnman::ThreadMessageHandler (this=0x555556892630) at ./src/net.cpp:2015
#13 0x00005555556259c4 in operator() (__closure=0x7fff0ebdba80) at ./src/net.cpp:2357
#14 0x0000555555635060 in std::__invoke_impl<void, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()>&>(std::__invoke_other, struct {...} &) (__f=...) at /usr/include/c++/11/bits/invoke.h:61
#15 0x00005555556344e5 in std::__invoke_r<void, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()>&>(struct {...} &) (__fn=...) at /usr/include/c++/11/bits/invoke.h:111
#16 0x0000555555633a98 in std::_Function_handler<void(), CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()> >::_M_invoke(const std::_Any_data &) (__functor=...) at /usr/include/c++/11/bits/std_function.h:290
#17 0x00005555557c6d39 in std::function<void ()>::operator()() const (this=0x7fff0ebdba80) at /usr/include/c++/11/bits/std_function.h:590
#18 0x00005555560a7b10 in util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>) (thread_name=\"msghand\", thread_func=...) at util/thread.cpp:21
#19 0x00005555556328d1 in std::__invoke_impl<void, void (*)(std::basic_string_view<char>, std::function<void()>), char const*, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()> >(std::__invoke_other, void (*&&)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>)) (__f=@0x5555612cfcf8: 0x5555560a7998 <util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>)>) at /usr/include/c++/11/bits/invoke.h:61
#20 0x0000555555631ff8 in std::__invoke<void (*)(std::basic_string_view<char>, std::function<void()>), char const*, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()> >(void (*&&)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>)) (__fn=@0x5555612cfcf8: 0x5555560a7998 <util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>)>) at /usr/include/c++/11/bits/invoke.h:96
#21 0x0000555555631ac8 in std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()> > >::_M_invoke<0, 1, 2>(std::_Index_tuple<0, 1, 2>) (this=0x5555612cfce8) at /usr/include/c++/11/bits/std_thread.h:253
#22 0x00005555556318bf in std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()> > >::operator()(void) (this=0x5555612cfce8) at /usr/include/c++/11/bits/std_thread.h:260
#23 0x000055555563172b in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()> > > >::_M_run(void) (this=0x5555612cfce0) at /usr/include/c++/11/bits/std_thread.h:211
#24 0x00007ffff7c192b3 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
#25 0x00007ffff78a0b43 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
#26 0x00007ffff7932a00 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

With your patch running, the node did sync through block 774628, which contains one of these (it's the record-largest transaction) without hitting the breakpoint. So that's good, it didn't reject the block.

@luke-jr
Copy link
Author

luke-jr commented Feb 2, 2023

If you up to the AcceptToMemoryPool frame, I think you can print tx->GetHash().ToString().c_str() or something to see the txid involved; then later, see if your node accepted or rejected it (debug.log or getrawtransaction). Another possibility for testing would be to find a tx and try feeding it into sendrawtransaction.

@LarryRuane
Copy link

Thanks, I did the print you suggested, and here some transactions that your patch detected (I could get more if needed):
https://mempool.space/tx/a489ac6c63341f79e748a07d32a7abbbff902440b8e26eafabbdcd1619887a7a
https://mempool.space/tx/1991330ee1c127e1c6c1c24cadc2b47604a4e7d9d67c6606f5eecd767e3be815
https://mempool.space/tx/86539902e20780b1ed39d9db96f42b076260a8e4294505b5ec667a7eb7577413

getrawtransaction doesn't find any of these.

sendrawtransaction on txid a489... errors with "Argument list too long". (They're all pretty large, so I don't think the others will work either). The hex is about 1.8 MB. Seems like there should be a way to pass the hex on stdin, maybe I'll make a PR.

@luke-jr
Copy link
Author

luke-jr commented Feb 3, 2023

Maybe use the Python console w/ a JSON-RPC lib to make the requests?

@LarryRuane
Copy link

Good idea, that worked, generated the following error:

bitcoinrpc.authproxy.JSONRPCException: -26: non-mandatory-script-verify-flag (NOPx reserved for soft-fork upgrades)

Here's the program I used:

#!/usr/bin/env python3
from bitcoinrpc.authproxy import AuthServiceProxy
import json
api = AuthServiceProxy("http://lmr:lmr@127.0.0.1:8332")

with open('rawtx', 'r') as file:
    tx = file.read()
# remove the trailing newline
tx = tx[:-1]
api.sendrawtransaction(tx)

(the file rawtx contains the hex of the ordinals transaction, which I grabbed from the mempool.space explorer)

Nothing appears in debug.log when I do this, even with all logging categories enabled.

@theStack
Copy link

theStack commented Feb 5, 2023

FWIW, here's another version that works for me, potentially allowing to specify more precise prefixes without having to nest GetOp calls:

diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 03b157a847..aaabcd759f 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -479,6 +479,11 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
                     return set_error(serror, SCRIPT_ERR_MINIMALDATA);
                 }
                 stack.push_back(vchPushValue);
+                const valtype ord_prefix{OP_FALSE, OP_IF, 0x03, 'o', 'r', 'd'};
+                if ((flags & SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS) && opcode == ord_prefix[0] &&
+                    std::mismatch(ord_prefix.begin()+1, ord_prefix.end(), pc, pend).first == ord_prefix.end()) {
+                    return set_error(serror, SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS);
+                }
             } else if (fExec || (OP_IF <= opcode && opcode <= OP_ENDIF))
             switch (opcode)
             {

@peterzion45
Copy link

f9a3766eb22a2f84d5bd67572f2e9e4dc541d7ca016242f36f02b38aad09cf5f

@RandyMcMillan
Copy link

These filters are an excercise in futility.

The inscription/ordinal convention can be changed faster than any Bitcoin release cadence.

Which leaves only a small subset of node runners willing to enforce new inscription/ordinal convention filters - probably having to compile in these filters on a daily basis.

Alternately - trying to make the filters more general to catch all possible patterns is beyond dangerous - it is absurd.

@luke-jr
Copy link
Author

luke-jr commented Feb 8, 2023

The inscription/ordinal convention can be changed faster than any Bitcoin release cadence.

But much slower than spam filters can be updated. And this is just a hypothetical issue; spammers typically give up quickly.

Which leaves only a small subset of node runners willing to enforce new inscription/ordinal convention filters

That doesn't follow either.

@VegArchie
Copy link

Anyone want to offer a noob advice on how to add this to my umbrel node?

@LarryRuane
Copy link

LarryRuane commented Feb 11, 2023

@peterzion45
Copy link

Ning makinabang kareni?

@garlonicon
Copy link

What about OP_TRUE OP_NOTIF? What about a lot of other ways to push data?

@peterzion45
Copy link

1b9058c84af7c6b05a76cc495e502d424e7eb41e45ec4c6209fff8cb2f2dbed1

@luke-jr
Copy link
Author

luke-jr commented Feb 17, 2023

@garlonicon This patch is just a quick hack thrown together to address the active attack in the safest and simplest way possible. It isn't meant to be a magic solution to all possible future problems.

@peterzion45
Copy link

ff875a4a6d237aad12e6ade232ece459cf8d80067702ffcdb6b73c3f58e60db2

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