Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions contrib/codeql/src/nightly/DecoderOutputBufferTooSmall.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* @name Decoder output buffer too small
* @description Finds decoder calls where the output argument is a
* fixed-size array smaller than the decoder's maximum
* documented write size.
* @kind problem
* @id asymmetric-research/decoder-output-buffer-too-small
* @problem.severity warning
* @precision high
* @tags reliability
* security
* external/cwe/cwe-121
*/

import cpp
import filter

private predicate constantIntegralValue(Expr e, int value) {
value = e.getValue().toInt()
}

private predicate fdBase64DecodedMax(Expr encodedSz, int decodedMax) {
exists(int n |
constantIntegralValue(encodedSz, n) and
n >= 0 and
decodedMax = ((n + 3) / 4) * 3
)
}

private predicate arrayByteSize(ArrayType arrayType, int bytes) {
bytes = arrayType.getArraySize() * arrayType.getBaseType().getSize()
}

private predicate outputBufferByteSize(Expr output, int bytes) {
exists(ArrayType arrayType |
arrayType = output.getType().getUnspecifiedType() and
arrayByteSize(arrayType, bytes)
)
or
exists(AddressOfExpr addressOf, ArrayType arrayType |
output = addressOf and
arrayType = addressOf.getOperand().getType().getUnspecifiedType() and
arrayByteSize(arrayType, bytes)
)
}

abstract class DecoderCall extends FunctionCall {
abstract Expr getOutputArgument();
abstract int getRequiredOutputBytes();
}

class Base64DecoderCall extends DecoderCall {
Base64DecoderCall() {
this.getTarget().hasName("fd_base64_decode") and
this.getNumberOfArguments() = 3
}

override Expr getOutputArgument() { result = this.getArgument(0) }

override int getRequiredOutputBytes() {
fdBase64DecodedMax(this.getArgument(2), result)
}
}

class FixedSizeBase58DecoderCall extends DecoderCall {
FixedSizeBase58DecoderCall() {
this.getTarget().hasName(["fd_base58_decode_32", "fd_base58_decode_64"]) and
this.getNumberOfArguments() = 2
}

override Expr getOutputArgument() { result = this.getArgument(1) }

override int getRequiredOutputBytes() {
(
this.getTarget().hasName("fd_base58_decode_32") and result = 32
)
or
(
this.getTarget().hasName("fd_base58_decode_64") and result = 64
)
}
}

from DecoderCall call, int outputBytes, int requiredBytes
where
outputBufferByteSize(call.getOutputArgument(), outputBytes) and
requiredBytes = call.getRequiredOutputBytes() and
outputBytes < requiredBytes and
included(call.getLocation())
select call.getOutputArgument(),
"The output buffer is " + outputBytes + " bytes, but $@ can write up to " +
requiredBytes + " bytes.",
call.getTarget(), call.getTarget().getName()
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
typedef unsigned char uchar;
typedef unsigned long ulong;

#define FD_BASE64_DEC_SZ(sz) ((((sz)+3UL)/4UL)*3UL)

long fd_base64_decode ( uchar * out, char const * in, ulong in_sz );
uchar * fd_base58_decode_32( char const * encoded, uchar * out );
uchar * fd_base58_decode_64( char const * encoded, uchar * out );

struct holder {
uchar out[ 16 ];
};

void
test_base64( char const * in,
ulong dynamic_sz ) {
uchar too_small[ 16 ];
fd_base64_decode( too_small, in, 24UL ); /* $ Alert */

uchar exact[ FD_BASE64_DEC_SZ( 24UL ) ];
fd_base64_decode( exact, in, 24UL );

uchar larger[ 32 ];
fd_base64_decode( larger, in, 40UL );

uchar unknown_needed[ 16 ];
fd_base64_decode( unknown_needed, in, dynamic_sz );

struct holder h;
fd_base64_decode( h.out, in, 24UL ); /* $ Alert */
}

void
test_base58( char const * in ) {
uchar too_small_32[ 31 ];
fd_base58_decode_32( in, too_small_32 ); /* $ Alert */

uchar exact_32[ 32 ];
fd_base58_decode_32( in, exact_32 );

uchar too_small_64[ 63 ];
fd_base58_decode_64( in, too_small_64 ); /* $ Alert */

uchar exact_64[ 64 ];
fd_base58_decode_64( in, exact_64 );
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
query: DecoderOutputBufferTooSmall.ql
postprocess: InlineExpectationsTestQuery.ql
Loading