Skip to content

Commit

Permalink
Fix SMB_COM_TRANSACTION payload padding (SMB1, #221)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mbechler
mbechler committed May 17, 2020
1 parent 2b1503f commit 593b674
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 40 deletions.
83 changes: 44 additions & 39 deletions src/main/java/jcifs/internal/smb1/trans/SmbComTransaction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ public abstract class SmbComTransaction extends ServerMessageBlock implements En
static final int DISCONNECT_TID = 0x01;
static final int ONE_WAY_TRANSACTION = 0x02;

static final int PADDING_SIZE = 2;
static final int PADDING_SIZE = 4;

private int tflags = 0x00;
private int pad = 0;
private int pad1 = 0;
private int pad2 = 0;
private boolean hasMore = true;
private boolean isPrimary = true;
private int bufParameterOffset;
Expand Down Expand Up @@ -219,18 +219,21 @@ public SmbComTransaction nextElement () {
if ( this.isPrimary ) {
this.isPrimary = false;

this.parameterOffset = this.primarySetupOffset + ( this.setupCount * 2 ) + 2;
if ( this.getCommand() != SMB_COM_NT_TRANSACT ) {
if ( this.getCommand() == SMB_COM_TRANSACTION && isResponse() == false ) {
this.parameterOffset += stringWireLength(this.name, this.parameterOffset);
}
}
else if ( this.getCommand() == SMB_COM_NT_TRANSACT ) {
this.parameterOffset += 2;
// primarySetupOffset
// SMB_COM_TRANSACTION: 61 = 32 SMB header + 1 (word count) + 28 (fixed words)
// SMB_COM_NT_TRANSACTION: 69 = 32 SMB header + 1 (word count) + 38 (fixed words)
this.parameterOffset = this.primarySetupOffset;

// 2* setupCount
this.parameterOffset += this.setupCount * 2;
this.parameterOffset += 2; // ByteCount

if ( this.getCommand() == SMB_COM_TRANSACTION && isResponse() == false ) {
this.parameterOffset += stringWireLength(this.name, this.parameterOffset);
}
this.pad = this.parameterOffset % getPadding();
this.pad = this.pad == 0 ? 0 : getPadding() - this.pad;
this.parameterOffset += this.pad;

this.pad1 = pad(this.parameterOffset);
this.parameterOffset += this.pad1;

this.totalParameterCount = writeParametersWireFormat(this.txn_buf, this.bufParameterOffset);
this.bufDataOffset = this.totalParameterCount; // data comes right after data
Expand All @@ -240,9 +243,8 @@ else if ( this.getCommand() == SMB_COM_NT_TRANSACT ) {
available -= this.parameterCount;

this.dataOffset = this.parameterOffset + this.parameterCount;
this.pad1 = this.dataOffset % getPadding();
this.pad1 = this.pad1 == 0 ? 0 : getPadding() - this.pad1;
this.dataOffset += this.pad1;
this.pad2 = this.pad(this.dataOffset);
this.dataOffset += this.pad2;

this.totalDataCount = writeDataWireFormat(this.txn_buf, this.bufDataOffset);

Expand All @@ -259,26 +261,24 @@ else if ( this.getCommand() == SMB_COM_NT_TRANSACT ) {

this.parameterOffset = SECONDARY_PARAMETER_OFFSET;
if ( ( this.totalParameterCount - this.parameterDisplacement ) > 0 ) {
this.pad = this.parameterOffset % getPadding();
this.pad = this.pad == 0 ? 0 : getPadding() - this.pad;
this.parameterOffset += this.pad;
this.pad1 = this.pad(this.parameterOffset);
this.parameterOffset += this.pad1;
}

// caclulate parameterDisplacement before calculating new parameterCount
this.parameterDisplacement += this.parameterCount;

int available = this.maxBufferSize - this.parameterOffset - this.pad;
int available = this.maxBufferSize - this.parameterOffset - this.pad1;
this.parameterCount = Math.min(this.totalParameterCount - this.parameterDisplacement, available);
available -= this.parameterCount;

this.dataOffset = this.parameterOffset + this.parameterCount;
this.pad1 = this.dataOffset % getPadding();
this.pad1 = this.pad1 == 0 ? 0 : getPadding() - this.pad1;
this.dataOffset += this.pad1;
this.pad2 = this.pad(this.dataOffset);
this.dataOffset += this.pad2;

this.dataDisplacement += this.dataCount;

available -= this.pad1;
available -= this.pad2;
this.dataCount = Math.min(this.totalDataCount - this.dataDisplacement, available);
}
if ( ( this.parameterDisplacement + this.parameterCount ) >= this.totalParameterCount
Expand All @@ -289,6 +289,18 @@ else if ( this.getCommand() == SMB_COM_NT_TRANSACT ) {
}


/**
* @return
*/
protected int pad ( int offset ) {
int p = offset % getPadding();
if ( p == 0 ) {
return 0;
}
return getPadding() - p;
}


/**
*
* @return padding size
Expand Down Expand Up @@ -350,32 +362,25 @@ protected int writeParameterWordsWireFormat ( byte[] dst, int dstIndex ) {
@Override
protected int writeBytesWireFormat ( byte[] dst, int dstIndex ) {
int start = dstIndex;
int p = this.pad;

if ( this.getCommand() == SMB_COM_TRANSACTION && isResponse() == false ) {
dstIndex += writeString(this.name, dst, dstIndex);
}

if ( this.parameterCount > 0 ) {
while ( p-- > 0 ) {
dst[ dstIndex++ ] = (byte) 0x00; // Pad
}
int end = dstIndex + this.pad1;

System.arraycopy(this.txn_buf, this.bufParameterOffset, dst, dstIndex, this.parameterCount);
dstIndex += this.parameterCount;
if ( this.parameterCount > 0 ) {
System.arraycopy(this.txn_buf, this.bufParameterOffset, dst, this.headerStart + this.parameterOffset, this.parameterCount);
end = Math.max(end, this.headerStart + this.parameterOffset + this.parameterCount + this.pad2);
}

if ( this.dataCount > 0 ) {
p = this.pad1;
while ( p-- > 0 ) {
dst[ dstIndex++ ] = (byte) 0x00; // Pad1
}
System.arraycopy(this.txn_buf, this.bufDataOffset, dst, dstIndex, this.dataCount);
System.arraycopy(this.txn_buf, this.bufDataOffset, dst, this.headerStart + this.dataOffset, this.dataCount);
this.bufDataOffset += this.dataCount;
dstIndex += this.dataCount;
end = Math.max(end, this.headerStart + this.dataOffset + this.dataCount);
}

return dstIndex - start;
return end - start;
}


Expand Down Expand Up @@ -417,7 +422,7 @@ public String toString () {
+ Hexdump.toHexString(this.tflags, 2) + ",timeout=" + this.timeout + ",parameterCount=" + this.parameterCount
+ ",parameterOffset=" + this.parameterOffset + ",parameterDisplacement=" + this.parameterDisplacement + ",dataCount="
+ this.dataCount + ",dataOffset=" + this.dataOffset + ",dataDisplacement=" + this.dataDisplacement + ",setupCount="
+ this.setupCount + ",pad=" + this.pad + ",pad1=" + this.pad1);
+ this.setupCount + ",pad=" + this.pad1 + ",pad1=" + this.pad2);
}

}
2 changes: 1 addition & 1 deletion src/main/java/jcifs/internal/smb1/trans/nt/SmbComNtTransaction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
public abstract class SmbComNtTransaction extends SmbComTransaction {

// relative to headerStart
private static final int NTT_PRIMARY_SETUP_OFFSET = 69;
private static final int NTT_PRIMARY_SETUP_OFFSET = 71;
private static final int NTT_SECONDARY_PARAMETER_OFFSET = 51;

/**
Expand Down

0 comments on commit 593b674

Please sign in to comment.