Please help with xp_cmdshell

Question

Please help with xp_cmdshell

Jeffery Williams

SSCertifiable

Points: 6441
More actions
December 11, 2017 at 8:16 am

#402740

Ok I am writing a proc that will look to a directory and import excel files. It is a work in progress overall however presently I am not getting past even the initial xp_cmdshell call.
When this is run I get the following error:
This is the string that is attempting to execute with EXEC master..xp_cmdshell @cmd
@cmd: 'dir /B g:\UnZip\*.*'
This is the result:
(0 row(s) affected)
Msg 8152, Level 16, State 2, Procedure xp_cmdshell, Line 29
String or binary data would be truncated.
This is the stored procedure. Presently I am just running by highlight and F5
/****** Object: StoredProcedure [dbo].[ImportFiles] Script Date: 12/11/2017 8:16:28 AM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER OFF
GO
ALTER PROCEDURE [dbo].[ImportFiles]
   @FilePath varchar(1000) = 'd:\UnZip\'
  ,@ArchivePath varchar(1000) = 'd:\Imported\'
  ,@FileNameMask varchar(1000) = '*.*'
  ,@MergeProc varchar(128) = 'MergeBCPData'
AS
SET NOCOUNT ON
-- Consider CALLING this SP from another SP that will pass along the CustomerID. This will allow for a managing
-- SP to determine which clients need to run.
--EXEC sp_configure 'show advanced options', 1
--EXEC sp_configure 'ad hoc distributed queries', 1
--RECONFIGURE
--RECONFIGURE
--GO
--EXEC sp_configure 'xp_cmdshell', 1
--GO
--RECONFIGURE
DECLARE      @ImportDate as datetime
            ,@CustomerID as int
            ,@SourceFileDIR as varchar(1000)

--/*
-- BElow is for out of band testing. Comment to commit proc and run production
DECLARE @FilePath varchar(1000) = 'g:\UnZip\'
   ,@ArchivePath varchar(1000) = 'g:\UnZip\Imported\'
   ,@FileNameMask varchar(1000) = '*.*'
   ,@MergeProc varchar(128) = 'MergeBCPData'
--*/

SELECT @ImportDate=getdate ()
DECLARE @FileName varchar(1000)
   ,@File varchar(8000)
DECLARE @cmd varchar(2000)
CREATE TABLE ##Import
  (
   s varchar(8000))
CREATE TABLE #Dir
  (s varchar(30) NULL)

    /*****************************************************************/
    -- Import file
    /*****************************************************************/
SELECT @cmd='dir /B '+@FilePath+@FileNameMask
SELECT @CMD = '''' + @CMD + ''''
PRINT '@cmd: ' + cast(@cmd as varchar(2000))
PRINT ''
DELETE #Dir
INSERT #Dir
EXEC master..xp_cmdshell @cmd
-- Test
--            EXEC master..xp_cmdshell dir /B g:\UnZip\*.*
DELETE #Dir
WHERE s IS NULL
   OR s LIKE '%not found%'
   OR s IN (SELECT ImportedFiles FROM ImportedFiles)

                     SELECT s as 'FileToImport'
                    FROM #Dir



WHILE EXISTS (SELECT * FROM #Dir WHERE s IS NOT NULL)
  BEGIN
    SELECT @FileName= min(s) FROM #Dir WHERE #Dir.s NOT IN (SELECT ImportedFiles FROM ImportedFiles)
    SELECT @File=@FilePath+@FileName

        PRINT '@File: ' + cast(@File as varchar(1000))
        PRINT''
    SELECT @cmd='bulk insert'
    SELECT @cmd=@cmd+' ##Import'
    SELECT @cmd=@cmd+' from'
    SELECT @cmd=@cmd+' '''+replace (@File,'"','')+''''
    SELECT @cmd=@cmd+' with (FIELDTERMINATOR=''|'''
    SELECT @cmd=@cmd+',ROWTERMINATOR = '''+char (10)+''')'



        PRINT 'Fully Built Command Line: ' + cast(@cmd as varchar(1000))
        PRINT''

    TRUNCATE TABLE ##Import

        -- import the data
    EXEC (@cmd)

        -- remove filename just imported
    DELETE #Dir
    WHERE s = @FileName

        INSERT INTO ImportedFiles
            (CustomerID, ImportedFiles)
        VALUES (@CustomerID, @FileName)

            EXEC @MergeProc


        -- Archive the file
    SELECT @cmd='move '+@FilePath+@FileName+' '+@ArchivePath+@FileName
    EXEC master..xp_cmdshell @cmd
  END

    IF @@ERROR = 0
    BEGIN TRANSACTION

            -- INSERT FIle Delete code here to remove imported files from the hard drive.
            -- xp_cmdshell 'del c:\delete-me-file.txt'

    COMMIT TRANSACTION
DROP TABLE ##Import
DROP TABLE #Dir
-- Sample code (NOT TESTED) To Delete files from FIle System after import: xp_cmdshell 'del c:\delete-me-file.txt'
<hr noshade size=1 width=250 color=#BBC8E5> Regards,Jeffery Williams http://www.linkedin.com/in/jwilliamsoh

Viewing 12 posts - 1 through 11 (of 11 total)

You must be logged in to reply to this topic. Login to reply

Bert-701015 SSCommitted Points: 1615 More actions · Answer 1

Have you considered using xp_dirtree instead of xp_cmdshell. It carries less risk. To your question/error, the length of the in column in your temp table (#Dir) is too small.

Jeffery Williams SSCertifiable Points: 6441 More actions · Answer 2

Bert-701015 - Monday, December 11, 2017 8:37 AM
Have you considered using xp_dirtree instead of xp_cmdshell. It carries less risk. To your question/error, the length of the in column in your temp table (#Dir) is too small.

Thank you so much I literally just fixed that. Now I have other errors. And yes I may switch to Dirtree, I just looked that up as well.

Do you see anything else glaring? And thank you so much for taking the time.

I now have these errors coming in, and I changed length to 300 and will tighten that up later.

Here is what I get now.

Msg 156, Level 15, State 1, Line 29
Incorrect syntax near the keyword 'is'.
Msg 105, Level 15, State 1, Line 30
Unclosed quotation mark after the character string ')'.

(1 row(s) affected)

(1 row(s) affected)
Msg 208, Level 16, State 1, Procedure MergeBCPData, Line 36
Invalid object name 'BCPData'.
(2 row(s) affected)
@File: g:\UnZip\operable program or batch file.

Fully Built Command Line: bulk insert ##Import from 'g:\UnZip\operable program or batch file.' with (FIELDTERMINATOR='|',ROWTERMINATOR = '
')

Msg 4860, Level 16, State 1, Line 29
Cannot bulk load. The file "g:\UnZip\operable program or batch file." does not exist.

(1 row(s) affected)
Msg 515, Level 16, State 2, Line 116
Cannot insert the value NULL into column 'CustomerID', table 'PointDev.dbo.ImportedFiles'; column does not allow nulls. INSERT fails.
The statement has been terminated.
Msg 208, Level 16, State 1, Procedure MergeBCPData, Line 36
Invalid object name 'BCPData'.

<hr noshade size=1 width=250 color=#BBC8E5> Regards,Jeffery Williams http://www.linkedin.com/in/jwilliamsoh

John Mitchell-245523 SSC Guru Points: 148809 More actions · Answer 3

Are any of the file names longer than the 30 characters you've allowed for in the #Dir table?

John

Jeffery Williams SSCertifiable Points: 6441 More actions · Answer 4

John Mitchell-245523 - Monday, December 11, 2017 8:51 AM
Are any of the file names longer than the 30 characters you've allowed for in the #Dir table?
John

The only table in my directory is a test excel file called' Book1'

<hr noshade size=1 width=250 color=#BBC8E5> Regards,Jeffery Williams http://www.linkedin.com/in/jwilliamsoh

Jeff Moden SSC Guru Points: 1003894 More actions · Answer 5

Bert-701015 - Monday, December 11, 2017 8:37 AM
Have you considered using xp_dirtree instead of xp_cmdshell. It carries less risk.

Actually, it doesn't. Proper use of xp_CmdShell carries no extra risk and disabling it provides no extra protection from those that could use it.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

ZZartin SSC-Dedicated Points: 30898 More actions · Answer 6

Jeff Moden - Monday, December 11, 2017 9:07 AM
Bert-701015 - Monday, December 11, 2017 8:37 AM
Have you considered using xp_dirtree instead of xp_cmdshell. It carries less risk.
Actually, it doesn't. Proper use of xp_CmdShell carries no extra risk and disabling it provides no extra protection from those that could use it.

It carries the risk of being harassed by ignorant auditors 😛

Jeffery Williams SSCertifiable Points: 6441 More actions · Answer 7

When I run as a proc I now get this for table list output.

<hr noshade size=1 width=250 color=#BBC8E5> Regards,Jeffery Williams http://www.linkedin.com/in/jwilliamsoh

Jeffery Williams SSCertifiable Points: 6441 More actions · Answer 8

ZZartin - Monday, December 11, 2017 9:09 AM
Jeff Moden - Monday, December 11, 2017 9:07 AM
Bert-701015 - Monday, December 11, 2017 8:37 AM
Have you considered using xp_dirtree instead of xp_cmdshell. It carries less risk.
Actually, it doesn't. Proper use of xp_CmdShell carries no extra risk and disabling it provides no extra protection from those that could use it.
It carries the risk of being harassed by ignorant auditors 😛

Truth.

<hr noshade size=1 width=250 color=#BBC8E5> Regards,Jeffery Williams http://www.linkedin.com/in/jwilliamsoh

John Mitchell-245523 SSC Guru Points: 148809 More actions · Answer 9

Jeffery Williams - Monday, December 11, 2017 9:17 AM
When I run as a proc I now get this for table list output.

Probably because in this line: SELECT @CMD = '''' + @CMD + '''' you padded the command with too many quotes. And that error could well be the reason that your getting the truncation message - it's longer than 30 characters.

John

Jeffery Williams SSCertifiable Points: 6441 More actions · Answer 10

John Mitchell-245523 - Monday, December 11, 2017 9:25 AM
Jeffery Williams - Monday, December 11, 2017 9:17 AM
When I run as a proc I now get this for table list output.
Probably because in this line: SELECT @CMD = '''' + @CMD + '''' you padded the command with too many quotes. And that error could well be the reason that your getting the truncation message - it's longer than 30 characters.
John

I changed the code. If I run the xp_cmdshell and string generated manually works. In the proc it does not. Here is the new code:

/****** Object: StoredProcedure [dbo].[ImportFiles] Script Date: 12/11/2017 8:16:28 AM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER OFF
GO
ALTER PROCEDURE [dbo].[ImportFiles]
   @FilePath varchar(1000) = 'g:\UnZip\'
  ,@ArchivePath varchar(1000) = 'g:\Imported\'
  ,@FileNameMask varchar(1000) = '*.*'
  ,@MergeProc varchar(128) = 'MergeBCPData'
AS

SET NOCOUNT ON

-- Consider CALLING this SP from another SP that will pass along the CustomerID. This will allow for a managing
-- SP to determine which clients need to run.

--EXEC sp_configure 'show advanced options', 1
--EXEC sp_configure 'ad hoc distributed queries', 1
--RECONFIGURE

--RECONFIGURE
--GO
--EXEC sp_configure 'xp_cmdshell', 1
--GO
--RECONFIGURE

DECLARE      @ImportDate as datetime
DECLARE         @CustomerID as int = 0
DECLARE         @SourceFileDIR as varchar(4000)

/*

-- Below is for out of band testing. Comment to commit proc and run production
DECLARE @FilePath varchar(1000) = 'g:\UnZip\'
   ,@ArchivePath varchar(1000) = 'g:\UnZip\Imported\'
   ,@FileNameMask varchar(1000) = '*.*'
   ,@MergeProc varchar(128) = 'MergeBCPData'

*/

SELECT @ImportDate=getdate ()

DECLARE @FileName varchar(1000)
,@File varchar(8000)

DECLARE @cmd varchar(2000)

CREATE TABLE ##Import
  (
   s varchar(8000))
CREATE TABLE #Dir
  (s varchar(300) NULL)

    /*****************************************************************/
    -- Import file
    /*****************************************************************/
SELECT @cmd= 'dir /B '+@FilePath+@FileNameMask

SELECT @CMD = '' + @CMD + ''''

PRINT '@cmd: ' + cast(@cmd as varchar(2000))
PRINT ''

EXEC master..xp_cmdshell @cmd

DELETE #Dir
INSERT #Dir
EXEC master..xp_cmdshell @cmd

SELECT '#DIR', *
FROM #DIR

-- Test
-- EXEC master..xp_cmdshell dir /B g:\UnZip\*.*

DELETE #Dir
WHERE s IS NULL
   OR s LIKE '%not found%'
   OR s IN (SELECT ImportedFiles FROM ImportedFiles)

WHILE EXISTS (SELECT * FROM #Dir WHERE s IS NOT NULL)
  BEGIN
    SELECT @FileName= min(s) FROM #Dir WHERE #Dir.s NOT IN (SELECT ImportedFiles FROM ImportedFiles)
    SELECT @File=@FilePath+@FileName

        PRINT '(line 95) @File: ' + cast(@File as varchar(1000))
        PRINT''

    SELECT @cmd='bulk insert'
    SELECT @cmd=@cmd+' ##Import'
    SELECT @cmd=@cmd+' from'
    SELECT @cmd=@cmd+' '''+replace (@File,'"','')+''''
    SELECT @cmd=@cmd+' with (FIELDTERMINATOR=''|'''
    SELECT @cmd=@cmd+',ROWTERMINATOR = '''+char (10)+''')'



        PRINT 'Fully Built Command Line: ' + cast(@cmd as varchar(1000))
        PRINT''

    TRUNCATE TABLE ##Import

        -- import the data
    EXEC (@cmd)

        -- remove filename just imported
    DELETE #Dir
    WHERE s = @FileName

        INSERT INTO ImportedFiles
            (CustomerID, ImportedFiles)
        VALUES (@CustomerID, @FileName)

            EXEC @MergeProc


        -- Archive the file
    SELECT @cmd='move '+@FilePath+@FileName+' '+@ArchivePath+@FileName
    EXEC master..xp_cmdshell @cmd
  END

    IF @@ERROR = 0
    BEGIN TRANSACTION

            -- INSERT FIle Delete code here to remove imported files from the hard drive.
            -- xp_cmdshell 'del c:\delete-me-file.txt'

    COMMIT TRANSACTION

DROP TABLE ##Import
DROP TABLE #Dir

-- Sample code (NOT TESTED) To Delete files from FIle System after import: xp_cmdshell 'del c:\delete-me-file.txt'

<hr noshade size=1 width=250 color=#BBC8E5> Regards,Jeffery Williams http://www.linkedin.com/in/jwilliamsoh

Jeff Moden SSC Guru Points: 1003894 More actions · Answer 11

ZZartin - Monday, December 11, 2017 9:09 AM
Jeff Moden - Monday, December 11, 2017 9:07 AM
Bert-701015 - Monday, December 11, 2017 8:37 AM
Have you considered using xp_dirtree instead of xp_cmdshell. It carries less risk.
Actually, it doesn't. Proper use of xp_CmdShell carries no extra risk and disabling it provides no extra protection from those that could use it.
It carries the risk of being harassed by ignorant auditors 😛

Heh... I rather think it will be the other way around if they try such a thing. I'll first show them the penetration tests, then make them sit through my hour long, demonstrative lecture on the subject, and then create a login for them to use and give them 24 hours to successfully execute xp_CmdShell... which, of course, they won't be able to do because only a limited number of people are allowed to have SA on the box. Then I'll also site (IIRC) rule T101 for auditors that states that they should know about what they're auditing instead of just following a script. 😉

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)